From 8f7d58cfea72c2c28670f3b7b59feead99b17987 Mon Sep 17 00:00:00 2001 From: RightL Date: Fri, 23 Oct 2026 20:24:49 +0800 Subject: [PATCH 1/3] Add native assistant execution with durable batches and protected completion Expose model-free JSON CLI and thin MCP entry points for native retrieval and Update tasks. Reuse candidate batching, isolated writes, validation, and Git queue coordination with durable claims and idempotent recovery. Add shared instructions, verification guidance, and focused regression coverage while retaining existing runtime entry points. --- DESIGN_NOTES.md | 29 ++ README.md | 7 +- docs/ASSISTANT_EXECUTION.md | 135 ++++++ docs/MCP.md | 4 + pyproject.toml | 1 + rightmemory/assistant.py | 517 ++++++++++++++++++++++ rightmemory/assistant_cli.py | 76 ++++ rightmemory/assistant_context.py | 105 +++++ rightmemory/assistant_mcp.py | 125 ++++++ rightmemory/assistant_prompt.py | 48 ++ rightmemory/assistant_write.py | 369 +++++++++++++++ rightmemory/async_update.py | 78 +++- rightmemory/cli.py | 32 +- rightmemory/entrypoint.py | 47 +- rightmemory/isolated_write.py | 44 +- rightmemory/prompts/assistant-retrieve.md | 19 + rightmemory/recent_submitted.py | 67 ++- rightmemory/update_queue.py | 4 +- rightmemory/update_queue_git.py | 24 +- rightmemory/validation.py | 98 ++++ skills/rightmemory-assistant/SKILL.md | 56 +++ tests/test_assistant_cli.py | 100 +++++ tests/test_assistant_context.py | 257 +++++++++++ tests/test_assistant_mcp.py | 234 ++++++++++ tests/test_assistant_queue.py | 203 +++++++++ tests/test_assistant_service.py | 227 ++++++++++ tests/test_assistant_service_review.py | 235 ++++++++++ tests/test_assistant_sync.py | 231 ++++++++++ tests/test_assistant_write.py | 308 +++++++++++++ tests/test_validation.py | 122 +++++ 30 files changed, 3729 insertions(+), 73 deletions(-) create mode 100644 docs/ASSISTANT_EXECUTION.md create mode 100644 rightmemory/assistant.py create mode 100644 rightmemory/assistant_cli.py create mode 100644 rightmemory/assistant_context.py create mode 100644 rightmemory/assistant_mcp.py create mode 100644 rightmemory/assistant_prompt.py create mode 100644 rightmemory/assistant_write.py create mode 100644 rightmemory/prompts/assistant-retrieve.md create mode 100644 rightmemory/validation.py create mode 100644 skills/rightmemory-assistant/SKILL.md create mode 100644 tests/test_assistant_cli.py create mode 100644 tests/test_assistant_context.py create mode 100644 tests/test_assistant_mcp.py create mode 100644 tests/test_assistant_queue.py create mode 100644 tests/test_assistant_service.py create mode 100644 tests/test_assistant_service_review.py create mode 100644 tests/test_assistant_sync.py create mode 100644 tests/test_assistant_write.py create mode 100644 tests/test_validation.py diff --git a/DESIGN_NOTES.md b/DESIGN_NOTES.md index f6d8ef3..7f195b0 100644 --- a/DESIGN_NOTES.md +++ b/DESIGN_NOTES.md @@ -1,5 +1,34 @@ # RightMemory Design Notes +## Native Assistant Execution + +Assistant execution separates the agent's judgment from RightMemory's deterministic +storage boundary. The shared orchestrator skill delegates retrieval needs and submits +evidence. A Retrieve child prepares access to live source files and pending evidence, +then summarizes what matters. An Update child uses the canonical Update instructions +and native file tools in a persistent Git worktree. CLI JSON and MCP call the same +model-free core; neither constructs an internal role agent. + +Batching reuses the existing candidate identity, reservation policy, and synchronized +Git queue. Readiness is carried in ordinary tool results, including instructions for a +child to relay the notice to its main agent. A notice does not acquire ownership. +Local assistant reservations survive individual CLI processes; synchronized claims use +a stable assistant-specific holder so the old worker cannot resume the same live lease. + +A batch retains its evidence identity across attempts. Each attempt has its own worktree, +version, and fenced semantic operation. Completion freezes committed edits, validates +every original commit and the complete candidate, retains the existing UpdateRecord, +and publishes through the existing write supervisor or Git queue finalizer. Prepared +results have an external finalizer so ordinary worker recovery cannot publish them +without their current claim. Durable outcomes recover interrupted acknowledgement; +failed publication retries the prepared result without repeating agent judgment. + +This is a separate execution path. It preserves the existing runtime and does not claim +equivalent model history, incremental retrieval caches, delivery suppression, watchers, +or maintenance scheduling. Ordinary sync is shared; semantic sync conflicts remain an +explicit blocked state until reconciled. Usage and verification are documented in +[Assistant Execution](docs/ASSISTANT_EXECUTION.md). + ## Embedding Retrieval The embedding backend separates finding likely context from deciding how to use it. Nemotron 1B selects forty candidates, Jina reranker v3.5 orders them, and the calling agent judges up to ten returned source entries. On the small frozen experiment, reranking recovered every labelled positive case within ten; score cutoffs also removed required context and failed on related questions whose answers were absent. The production path therefore uses rank and a count limit without claiming to detect missing answers. The experiment and its limits are recorded in `experiments/README.md`. diff --git a/README.md b/README.md index 4a78588..c116144 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,8 @@ RightMemory is aimed at developers who spend serious time with coding agents and ## Quick Start +To use your current assistant's native agents without a RightMemory model backend, follow [Assistant execution](docs/ASSISTANT_EXECUTION.md). It uses `rightmemory assistant`, a shared skill, and native file tools. The installer paths below configure the existing internal runtime. + Install prerequisites: ```bash @@ -96,9 +98,9 @@ rightmemory doctor agent-cli rightmemory doctor agent-cli ``` -Before using RightMemory, configure its role backend in `/rightmemory.toml` (by default `~/.rightmemory/rightmemory.toml`). See [Standalone Config](#standalone-config) for model endpoints and credentials, or [CLI-Agent Config](#cli-agent-config) for Codex SDK / Claude Code CLI execution. MCP connects your host agent to this runtime; it does not replace the configured backend or use the host's model automatically. +Before using the internal runtime, configure its role backend in `/rightmemory.toml` (by default `~/.rightmemory/rightmemory.toml`). See [Standalone Config](#standalone-config) for model endpoints and credentials, or [CLI-Agent Config](#cli-agent-config) for Codex SDK / Claude Code CLI execution. The `rightmemory mcp` transport connects your host agent to this runtime; it does not replace the configured backend or use the host's model automatically. Native assistant execution instead uses `rightmemory assistant`, including its optional `assistant mcp` transport. -For an MCP client, follow [MCP setup](docs/MCP.md) to register `rightmemory mcp` as a local stdio server. Connect the server in your host and check its available tools: `rightmemory_retrieve`, `rightmemory_submit_update`, and `rightmemory_capture_guidance`. Seeing all three verifies tool discovery, not backend execution or a completed memory update. Do not also load a RightMemory orchestrator skill in that MCP conversation; command-only clients use the CLI skills instead. +For the internal runtime's MCP transport, follow [MCP setup](docs/MCP.md) to register `rightmemory mcp` as a local stdio server. Connect the server in your host and check its available tools: `rightmemory_retrieve`, `rightmemory_submit_update`, and `rightmemory_capture_guidance`. Seeing all three verifies tool discovery, not backend execution or a completed memory update. Do not also load a RightMemory orchestrator skill in that MCP conversation; command-only clients use the CLI skills instead. After connecting MCP, add a short instruction to your agent guidance file, such as `AGENTS.md` for Codex or `CLAUDE.md` for Claude Code: @@ -171,6 +173,7 @@ For a short recording script, see [docs/DEMO.md](docs/DEMO.md). - Multi-device memory continuity across laptops, desktops, agent clients, and project-specific roots. - Automatic orchestration plus independent explicit skills for map maintenance, Memory maintenance, guidance review, and Manager support in Codex App. - Two executor modes behind the same `rightmemory` CLI: standalone runtime or delegated Codex SDK/Claude Code CLI role execution. +- Native assistant execution through shared role instructions and deterministic JSON CLI/MCP tools; your assistant supplies its own agents. - Source-authored retrieval output, selected by an agent or an optional embedding + reranking retrieval service. - One updater for Memory and Agent Corrections, transcript-review candidate extraction, and immutable candidate records for input-to-edit provenance. diff --git a/docs/ASSISTANT_EXECUTION.md b/docs/ASSISTANT_EXECUTION.md new file mode 100644 index 0000000..7af5183 --- /dev/null +++ b/docs/ASSISTANT_EXECUTION.md @@ -0,0 +1,135 @@ +# Assistant Execution + +RightMemory can use the current assistant's native agents for memory work. The main agent submits evidence and delegates ready batches. Retrieve inspects the real memory files and pending evidence; Update edits an isolated Git worktree. RightMemory performs deterministic queue, validation, publication, and recovery operations without creating a model client. + +This path uses one shared [assistant skill](../skills/rightmemory-assistant/SKILL.md), a [Retrieve role](../rightmemory/prompts/assistant-retrieve.md), and the existing [Update role](../rightmemory/prompts/update.md). The existing standalone and CLI-agent paths remain available. + +## Setup + +Install the package in a Python 3.11+ environment on the computer that will hold the memory root: + +```bash +python -m pip install . +``` + +Use a complete RightMemory Git root with a configured Git author. A disposable root is appropriate for initial verification. Copy `skills/rightmemory-assistant/` from the same checkout into the assistant's supported skill location and enable it in the conversation. The existing `install.sh` and `install.ps1` modes do not select assistant execution or install this skill automatically. + +Assistant execution does not require RightMemory model API credentials, Codex, or Claude Code. The assistant uses its own configured agent. Keep the active root explicit: + +```bash +rightmemory assistant --root /absolute/memory-root context --session example-session +``` + +The assistant's native subagents must be able to read the returned memory and pending paths and write the assigned Update worktree. A remote MCP connection does not expose the server's filesystem to native file tools. Place the runtime and native agents in a common accessible environment, or provide an authorized shared filesystem. Each assistant may use its own synchronized Git checkout. + +An Update worktree isolates candidate edits from the active root. A working-directory setting alone does not restrict arbitrary native shell access; use the assistant's file permissions where available. RightMemory validates the candidate it admits, while the assistant governs its agents' other file access. + +## Read and Submit + +The main agent delegates the retrieval need, bound root, session id, and `context` command (or matching MCP tool) to a native Retrieve subagent. That subagent calls `context` and reads its returned instruction file. The result supplies real source locations, imported-view mappings, and readable pending evidence, plus the observed `memory_commit` and `live_files: true`: memory sources remain live, rather than becoming a frozen snapshot. The subagent inspects them with native file tools and returns a useful summary with sources. This path does not require a `RetrieveSelection` or run the original retrieval renderer. + +Save submission evidence in a UTF-8 file outside the memory root: + +```bash +rightmemory assistant --root /absolute/memory-root submit --session example-session --request-id evidence-1 --file /absolute/evidence.txt +``` + +Submission records evidence for later reconciliation. It does not start an internal model worker. Retry the same submission with the same request id and content; changed evidence needs a new request id. + +Normal assistant tool responses can carry a batch-ready notice. A Retrieve or other subagent passes that notice to its main agent, which arranges Update. The program owns pending and active batch state; the main agent owns native delegation. + +## Update a Batch + +```bash +rightmemory assistant --root /absolute/memory-root update begin +``` + +Delegate only a newly claimed task (`claimed: true`). Give the native Update subagent the returned `instructions_file`, `candidates_file`, and `worktree`. Repeating ordinary `begin` while a task is active returns that task with `claimed: false`; resume the existing task instead of dispatching a second updater. `status: not_ready` means there is no eligible batch, and `status: sync_required` means sync needs attention first. + +Update follows the canonical role, including native edits, complete validation, and commits restricted to allowed files. `validate` remains independently callable: + +```bash +rightmemory validate --root /absolute/candidate-worktree +``` + +Complete the task using the returned completion command, preserving its active-root binding: + +```bash +rightmemory assistant --root /absolute/memory-root update complete --task TASK +rightmemory assistant --root /absolute/memory-root update status --task TASK +``` + +`complete` admits the exact validated candidate through the program's protected publication path and records the batch outcome. A commit inside the worktree is not yet an applied update. An unchanged reconciliation still needs completion. `--output-file FILE` can attach the agent's UTF-8 summary to completion. The completion receipt includes `landed_commit`, `changed_paths`, and a `sync` outcome of `published` or `disabled`. + +After an interrupted call or lost response, inspect the same task and retry its completion. Keep its identity and worktree. When shared Git sync is enabled, completion first durably prepares the candidate, then publishes through the shared queue's compare-and-swap finalizer. If publication fails, the prepared result remains available; repeating `complete` retries publication without repeating semantic Update. With sync disabled, completion uses the local protected write path. + +A stale-base conflict requires reconciliation against current state. To abandon the current task's edits and prepare the same batch again, use: + +```bash +rightmemory assistant --root /absolute/memory-root update begin --retry TASK +``` + +The replacement task fences the old task, so its edits cannot later be completed. The program checks for a previously completed or recoverable prepared result before replacing a task. Do not use replacement merely because a completion reply was lost. + +## Sync and MCP + +Configured ordinary sync refreshes memory before retrieval and pulls before preparing Update. Shared-queue completion publishes through the protected Git finalizer. Explicit ordinary sync operations use the same core: + +```bash +rightmemory assistant --root /absolute/memory-root sync refresh +rightmemory assistant --root /absolute/memory-root sync pull +rightmemory assistant --root /absolute/memory-root sync push +``` + +An incoming conflict or invalid merged state returns sync status and available file/operation details, and blocks a new Update. This assistant interface does not yet provide a native-agent sync-repair workspace protocol. The existing separate repair paths remain available; resolve the conflict before retrying ordinary sync. + +The optional MCP transport wraps the same assistant core: + +```bash +rightmemory assistant --root /absolute/memory-root mcp --transport stdio +``` + +Load the shared assistant skill with this MCP transport; it explains how the assistant's own agents use the deterministic tools. The older `rightmemory mcp` command continues to use its configured internal role backend. See [MCP](MCP.md). + +The assistant path is not a replacement for all internal-runtime behavior. It does not promise the old model-session history, incremental retrieval cache, delivery suppression, transcript watchers, Dreamer, or scheduled maintenance. Use the existing paths for those mechanisms unless separately supported. The public assistant queue and completion receipts cover native batch execution rather than pretending that the old `update submit` worker queue dispatches native subagents. + +## Dot Verification + +Run the following in an explicitly authorized disposable memory root. Do not connect real memory, create credentials, or enable a persistent connection merely to run this checklist. In that root's `rightmemory.toml`, use a small test batch so one submitted candidate becomes ready immediately: + +```toml +[update.async] +trigger_candidates = 1 +target_batch_candidates = 1 +max_wait_seconds = 1 +``` + +The normal defaults are 15 candidates to trigger, 30 as the preferred batch size, and a 24-hour fallback after the quiet period. The small values above are for this disposable exercise. + +1. **Check the actual execution environment.** Let a native Retrieve subagent read a fictional marker stored in a valid memory entry using its own file tools. During Update, let its native subagent modify an allowed entry in the assigned worktree, then have RightMemory inspect that candidate. This establishes shared filesystem access; MCP discovery alone does not. +2. **Exercise the shared skill.** Submit a fictional durable fact. Before reconciliation, have a fresh Retrieve subagent find it as pending evidence. Trigger a normal tool call that returns a ready-batch notice to the subagent and verify that the main agent receives it and delegates Update. +3. **Finish a real Update.** Let the native Update subagent read the candidate, decide and make the edit in the assigned worktree, run standalone validation, commit, and complete the task. Inspect the program's outcome and the active root's Git change. +4. **Read it independently.** Start a fresh Retrieve subagent with a question that requires the new fact, without giving it the answer or the Update transcript. Check the returned source against the committed memory. +5. **Exercise failure cases.** Run the cases below against disposable state and inspect both operation receipts and Git state. + +| Case | Required result | +| --- | --- | +| Same submission retried | The original submission is returned; evidence is not duplicated. Reusing its id for different content fails. | +| Active root changed after `begin` | Completion reports a conflict; it does not overwrite the newer state. | +| Invalid path or non-regular file | Candidate admission rejects unsafe Git paths or file types; task and session ids cannot escape their state directory. No outside path is changed through the program. | +| Invalid candidate graph | Independent validation and completion reject it; active memory stays unchanged. | +| Interrupted before application | The task remains inspectable and recoverable; no partial active update appears. | +| Applied, reply lost | Status or repeated completion reports the same outcome; no second memory commit is created. | +| Shared-queue publication failed | The durable prepared result remains; repeating completion retries publication without repeating semantic Update. | + +Record the assistant/version, RightMemory commit, root and worktree locations, native agent identities, task id, validation result, applied commit, and sync outcome. A check passes only when its observed output supports that result. + +## Assistant Verification Status + +| Assistant | What the existing evidence establishes | Still to verify for this path | +| --- | --- | --- | +| dot | An earlier isolated stdio handshake, three-tool discovery/error handling, and a limited synthetic write/read exercise. | The native-agent batch flow and failure/recovery cases above; native plugin and filesystem access in the intended deployment. | +| DeepSeek Harness | No completed test of this path is recorded. | This shared skill and tool flow in DSH, using its native provider and filesystem. | +| WorkBuddy | No completed test of this path in WorkBuddy itself is recorded. | The full flow in WorkBuddy itself, including native delegation and worktree access. CodeBuddy results do not establish this. | + +These are verification boundaries, not claims that a product has been adapted. Private dot MCP access may use [OpenAI Secure MCP Tunnel](https://developers.openai.com/api/docs/guides/secure-mcp-tunnels), including its direct stdio bridge; tunnel authentication and a persistent client require separate setup and authorization. diff --git a/docs/MCP.md b/docs/MCP.md index 6de9e40..f073be5 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -1,5 +1,9 @@ # RightMemory MCP +For native assistant execution, use `rightmemory assistant --root /absolute/memory-root mcp --transport stdio` with the shared `rightmemory-assistant` skill. This transport exposes the model-independent assistant core and returns native-agent handoffs and batch notices. See [Assistant execution](ASSISTANT_EXECUTION.md) for setup, filesystem access, and verification boundaries. + +The `rightmemory mcp` server described below keeps the existing internal role backend and its ordinary-agent contract. + RightMemory exposes a local MCP stdio server for ordinary agent work using the official MCP Python SDK v2: ```bash diff --git a/pyproject.toml b/pyproject.toml index dd5f266..977b1d0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,3 +54,4 @@ exclude = ["rightmemory/web/frontend/**"] [tool.hatch.build.targets.wheel.force-include] "MEMORY.example.md" = "rightmemory/MEMORY.example.md" "PURSUITS.example.md" = "rightmemory/PURSUITS.example.md" +"skills/rightmemory-assistant" = "rightmemory/skills/rightmemory-assistant" diff --git a/rightmemory/assistant.py b/rightmemory/assistant.py new file mode 100644 index 0000000..138d62d --- /dev/null +++ b/rightmemory/assistant.py @@ -0,0 +1,517 @@ +from __future__ import annotations + +import hashlib +import json +import os +import re +import shlex +import uuid +from contextlib import contextmanager +from dataclasses import asdict +from datetime import UTC, datetime, timedelta +from pathlib import Path +from typing import Any + +from .assistant_context import prepare_retrieve_context +from .assistant_prompt import build_assistant_update_instructions +from .assistant_write import AssistantWriteManager +from .async_update import AsyncUpdateStore +from .config import load_async_update_config, load_sync_config +from .isolated_write import IsolatedWriteResult +from .platform import lock_file, unlock_file +from .semantic_operation import FINAL_PHASES, SemanticOperationStore +from .session import _ensure_runtime_gitignore, _fsync_directory, _safe_session_id +from .sync import SyncManager +from .update_queue import ( + SCHEMA_VERSION, parse_update_candidate_data, parse_update_queue_lease_json, + update_candidate_batch_id, update_candidate_data, +) +from .update_queue_git import ( + QUEUE_FINALIZER, QUEUE_LEASE_SECONDS, ClaimedUpdateBatch, GitUpdateQueueCoordinator, + UpdateQueueLeaseLost, UpdateQueueSemanticBaseChanged, +) + + +class AssistantClaimLost(RuntimeError): + """The task was replaced; its edits cannot be published.""" + + +class AssistantService: + """Model-free entry points for native assistant tasks and durable batching.""" + + def __init__(self, memory_root: Path): + self.memory_root = Path(memory_root).expanduser().resolve() + if not self.memory_root.is_dir(): + raise FileNotFoundError(f"RightMemory memory root does not exist: {self.memory_root}") + self.root = self.memory_root / ".runtime" / "assistant" + _check_path(self.root) + self.store = AsyncUpdateStore(self.memory_root, "update", executor="assistant") + self.writer = AssistantWriteManager(self.memory_root) + self.sync_config = load_sync_config(self.memory_root) + config = load_async_update_config(self.memory_root) + self.batch_options = { + "trigger_candidates": config.trigger_candidates, + "target_batch_candidates": config.target_batch_candidates, + "max_wait_seconds": config.max_wait_seconds, + } + + def context(self, session_id: str) -> dict[str, Any]: + _safe_session_id(session_id) + # Completion and queue acknowledgement may have been interrupted. Only + # proven terminal outcomes are settled here; no model work is resumed. + with self._locked(): + self._settle_terminal_active() + queue = self._publish_outbox() + result = prepare_retrieve_context( + self.memory_root, excluded_candidate_uids=self._completed_candidates(), + ) + return self._response({"session_id": session_id, **result, "queue": queue}) + + def submit(self, session_id: str, request_id: str, evidence: str) -> dict[str, Any]: + _safe_session_id(session_id) + if not isinstance(request_id, str) or not request_id.strip(): + raise ValueError("request_id must be a non-empty stable submission identifier") + if not isinstance(evidence, str) or not evidence.strip(): + raise ValueError("evidence must not be empty") + uid = hashlib.sha256(json.dumps([session_id, request_id]).encode()).hexdigest()[:32] + path = self.root / "submissions" / f"{uid}.json" + with self._locked(): + identity = {"session_id": session_id, "request_id": request_id, "evidence": evidence} + previous = _read_json(path) + if previous is not None and previous != identity: + raise ValueError("request_id already belongs to different update evidence") + _write_json(path, identity) + state = self.store.submit(session_id, evidence, candidate_uid=uid) + self._settle_terminal_active() + queue = self._publish_outbox() + return self._response({ + "saved": True, "candidate_uid": uid, "duplicate": previous is not None, + "session_id": session_id, "session_status": state.status, "queue": queue, + }) + + def begin_update(self, retry_task: str | None = None) -> dict[str, Any]: + with self._locked(): + active = self._active() + if retry_task is not None: + requested = self._task(retry_task) + if requested.get("receipt") is not None: + return self._response(requested["receipt"]) + if active is None or active["task_id"] != retry_task: + raise AssistantClaimLost("only the current task can be retried") + if active is not None: + settled = self._settle_terminal_active() + if settled is None and active["backend"] == "git" and ( + retry_task is not None or _time(active["expires_at"]) <= _now() + ): + settled = self._settle_git_terminal(active) + if settled is not None: + if retry_task is not None: + return self._response(settled) + active = None + elif retry_task is None and _time(active["expires_at"]) > _now(): + return self._response(self._prepare(active, claimed=False)) + else: + # A prepared result might already have landed before its + # receipt. Recover it before permitting a replacement task. + operation = self._operation(active) + if operation is not None and operation.phase == "prepared": + try: + return self._response(self._complete(active)) + except (UpdateQueueSemanticBaseChanged, UpdateQueueLeaseLost): + pass + except RuntimeError as exc: + from .assistant_write import AssistantStaleVersionError + if not isinstance(exc, AssistantStaleVersionError): + raise + + sync_result = SyncManager(self.sync_config).pull() + if sync_result.status not in {"disabled", "synced", "ahead", "pushed"}: + return self._response({"status": "sync_required", "sync": asdict(sync_result)}) + publication = self._publish_outbox() + if self.sync_config.enabled: + result = self._coordinator().claim_next(**self.batch_options) + claim = result.claim + if claim is None: + return self._response({"status": "not_ready", "queue": publication, + "online": result.online}) + candidates = claim.candidates + batch_id = claim.batch_id + backend = "git" + else: + batch, _deadline = self.store.ready_batch(**self.batch_options) + if not batch: + return self._response({"status": "not_ready"}) + reserved = self.store.reserve_batch(batch) + if reserved is None: + return self._response({"status": "not_ready"}) + batch_id = reserved.operation_id + candidates = self.store.start_reserved_batch(batch_id) + claim = None + backend = "local" + if active is not None and active["batch_id"] != batch_id: + raise AssistantClaimLost("active batch was replaced; inspect update status before proceeding") + record = { + "task_id": uuid.uuid4().hex, "batch_id": batch_id, "backend": backend, + "created_at": _now().isoformat(), + "expires_at": (_now() + timedelta(seconds=QUEUE_LEASE_SECONDS)).isoformat(), + "candidates": [update_candidate_data(item) for item in candidates], + "claim": None if claim is None else { + "lease": asdict(claim.lease), "lease_commit": claim.lease_commit, + }, + "status": "running", + } + self._save_task(record) + _write_json(self.root / "active.json", {"task_id": record["task_id"]}) + if active is not None: + active["status"] = "superseded" + active["replacement_task"] = record["task_id"] + self._save_task(active) + return self._response(self._prepare(record, claimed=True)) + + def complete_update(self, task_id: str, output: str = "") -> dict[str, Any]: + with self._locked(): + record = self._task(task_id) + if record.get("receipt") is not None: + receipt = record["receipt"] + if receipt["status"] == "completed" and output and output != receipt["output"]: + raise ValueError("assistant completion already has a different output") + return self._response(receipt) + self._check_current(task_id) + return self._response(self._complete(record, output=output)) + + def update_status(self, task_id: str | None = None) -> dict[str, Any]: + with self._locked(): + self._settle_terminal_active() + record = self._task(task_id) if task_id is not None else self._active() + if record is None: + return self._response({"status": "idle"}) + if record.get("receipt") is not None: + return self._response(record["receipt"]) + status = {key: record[key] for key in ("task_id", "batch_id", "status", "expires_at")} + try: + task = self.writer.read(record["task_id"]) + status.update(worktree=str(task.worktree), base_commit=task.base_commit) + operation = self._operation(record) + status["completion_phase"] = None if operation is None else operation.phase + except FileNotFoundError: + status["completion_phase"] = "preparing" + status["complete_argv"] = self._argv("update", "complete", "--task", record["task_id"]) + return self._response(status) + + def sync(self, action: str = "refresh") -> dict[str, Any]: + manager = SyncManager(self.sync_config) + actions = {"refresh": manager.refresh_for_retrieve, "pull": manager.pull, "push": manager.push} + if action not in actions: + raise ValueError("sync action must be refresh, pull, or push") + result = actions[action]() + with self._locked(): + self._settle_terminal_active() + queue = self._publish_outbox() + return self._response({"sync": asdict(result), "queue": queue}) + + def _prepare(self, record: dict, *, claimed: bool) -> dict: + task_id = record["task_id"] + task = self.writer.prepare( + task_id, record["batch_id"], self._candidates(record), + check_current=lambda: self._check_current(task_id), + ) + folder = self.root / "tasks" / task_id + folder.mkdir(parents=True, exist_ok=True) + instructions = folder / "UPDATE.md" + instructions.write_text(build_assistant_update_instructions( + task.worktree, task_id, active_root=self.memory_root, + ), encoding="utf-8") + evidence = folder / "CANDIDATES.json" + _write_json(evidence, {"batch_id": task.batch_id, "candidates": record["candidates"]}) + argv = self._argv("update", "complete", "--task", task_id) + return { + "status": "ready" if claimed else "active", "claimed": claimed, + "task_id": task_id, "batch_id": task.batch_id, "worktree": str(task.worktree), + "base_commit": task.base_commit, "expires_at": record["expires_at"], + "instructions_file": str(instructions), "candidates_file": str(evidence), + "complete_argv": argv, "complete_command": shlex.join(argv), + } + + def _complete(self, record: dict, output: str = "") -> dict: + task_id = record["task_id"] + self._check_current(task_id) + operation = self._operation(record) + if operation is not None and operation.phase in FINAL_PHASES: + task = self.writer.read(task_id) + if output and output != task.output: + raise ValueError("assistant completion already has a different output") + return self._finish_terminal(record, operation) + result = self.writer.complete( + task_id, output=output, check_current=lambda: self._check_current(task_id), + external_finalizer=QUEUE_FINALIZER if record["backend"] == "git" else None, + ) + if record["backend"] == "git" and result.prepared: + self._check_current(task_id) + try: + landed = self._coordinator().finalize( + self._claim(record), result.candidate_commit, + prepared_start_commit=result.start_commit, + ) + except UpdateQueueLeaseLost: + settled = self._settle_git_terminal(record) + if settled is not None: + return settled + raise + result = self.writer.complete_external( + task_id, external_finalizer=QUEUE_FINALIZER, landed_commit=landed, + ) + return self._finish(record, result) + + def _finish(self, record: dict, result) -> dict: + if record["backend"] == "local": + self.store.finish_reserved_batch(record["batch_id"], str(result.output)) + else: + uids = tuple(item.uid for item in self._candidates(record)) + self.store.acknowledge_synchronized(set(uids), result=str(result.output)) + self._coordinator().clear_local_candidates(uids) + receipt = { + "status": "completed_elsewhere" if record.get("completed_elsewhere") else "completed", + "task_id": record["task_id"], "batch_id": record["batch_id"], + "landed_commit": result.landed_commit, "changed_paths": list(result.changed_paths), + "output": result.output, "sync": "published" if record["backend"] == "git" else "disabled", + } + record.update(status="completed", receipt=receipt) + self._save_task(record) + if self._active_id() == record["task_id"]: + _write_json(self.root / "active.json", {"task_id": None}) + # Keep a workspace with unexpected later edits; successful publication + # must not turn into a failed request merely because cleanup is deferred. + try: + self.writer.cleanup(record["task_id"]) + except (OSError, RuntimeError) as exc: + receipt["cleanup_warning"] = str(exc) + self._save_task(record) + return receipt + + def _settle_git_terminal(self, record: dict) -> dict | None: + coordinator = self._coordinator() + operation = self._operation(record) + own_landed = coordinator.fetch_and_integrate_finalized_batch( + record["batch_id"], token=self._claim(record).lease.token, + ) + if own_landed is not None and operation is not None and operation.phase == "prepared": + self._check_current(record["task_id"]) + result = self.writer.complete_external( + record["task_id"], external_finalizer=QUEUE_FINALIZER, landed_commit=own_landed, + ) + return self._finish(record, result) + landed = coordinator.fetch_and_integrate_finalized_batch(record["batch_id"]) + if landed is None: + landed = coordinator.fetch_and_integrate_superseded_batch(record["batch_id"]) + if landed is None: + return None + self._check_current(record["task_id"]) + store = SemanticOperationStore(self.memory_root) + if operation is not None: + if operation.phase == "prepared": + self.writer.supervisor.supersede_external( + operation.operation_id, external_finalizer=QUEUE_FINALIZER, landed_commit=landed, + ) + elif operation.phase == "running": + with store.execution_locked(): + store.supersede_running( + operation.operation_id, landed_commit=landed, + reason="batch settled by another synchronized queue owner", + ) + record["completed_elsewhere"] = True + return self._finish(record, IsolatedWriteResult( + output="batch already settled by the synchronized queue", commits_landed=0, + landed_commit=landed, + )) + + def _settle_terminal_active(self) -> dict | None: + record = self._active() + if record is None: + return None + if record.get("receipt") is not None: + _write_json(self.root / "active.json", {"task_id": None}) + return record["receipt"] + operation = self._operation(record) + if operation is not None and operation.phase in FINAL_PHASES: + return self._finish_terminal(record, operation) + return None + + def _finish_terminal(self, record: dict, operation) -> dict: + outcome = operation.outcome + if outcome is None: + raise RuntimeError("terminal assistant operation has no durable outcome") + if outcome.metadata.get("superseded"): + record["completed_elsewhere"] = True + return self._finish(record, IsolatedWriteResult( + output=outcome.output, commits_landed=int(operation.phase == "committed"), + start_commit=outcome.start_commit, landed_commit=outcome.landed_commit or "", + changed_paths=outcome.changed_paths, operation_id=operation.operation_id, + )) + + def _operation(self, record: dict): + try: + task = self.writer.read(record["task_id"]) + except FileNotFoundError: + return None + return SemanticOperationStore(self.memory_root).read(task.operation_id) + + def _publish_outbox(self) -> dict: + if not self.sync_config.enabled: + return {"status": "local"} + try: + coordinator = self._coordinator() + result = coordinator.publish_outbox() + if result.settled_uids: + pulled = SyncManager(self.sync_config).pull() + if pulled.status not in {"synced", "ahead"}: + # Keep the local evidence until its authoritative queue + # state is also visible to native Retrieve and notices. + return {**asdict(result), "status": "pending", "sync": asdict(pulled)} + self.store.acknowledge_synchronized(set(result.settled_uids)) + coordinator.clear_local_candidates(result.settled_uids) + return asdict(result) + except (OSError, RuntimeError, ValueError) as exc: + # Evidence is durable before network publication. A later normal + # call retries publication using the same candidate identity. + return {"status": "pending", "error": str(exc)} + + def _response(self, result: dict) -> dict: + response = {"ok": True, "active_root": str(self.memory_root), **result} + try: + response["update"] = self._notice() + except (OSError, ValueError, RuntimeError) as exc: + response["update"] = {"ready": False, "status": "error", "error": str(exc)} + return response + + def _notice(self) -> dict: + active = self._active() + if active is not None and active.get("receipt") is None: + expired = _time(active["expires_at"]) <= _now() + if not expired: + return {"ready": False, "status": "active", "task_id": active["task_id"]} + notice = {"ready": True, "status": "resume", "task_id": active["task_id"]} + else: + if self.sync_config.enabled: + candidates, deadline = self._coordinator().ready_candidates(**self.batch_options) + else: + batches, deadline = self.store.ready_batch(**self.batch_options) + candidates = tuple( + self.store._candidate_from_job(batch.session_id, job) + for batch in batches or () for job in batch.jobs + ) + if not candidates: + return {"ready": False, "status": "waiting", + "next_ready_at": None if deadline is None else deadline.isoformat()} + notice = {"ready": True, "status": "ready", "batch_id": update_candidate_batch_id(candidates), + "candidate_count": len(candidates)} + notice["begin_argv"] = self._argv("update", "begin") + notice["instruction"] = ( + "If you are a child agent or task, relay this entire update notice to the main agent " + "alongside your result. Main agent: delegate one Update child using the returned " + "begin command (or rightmemory_assistant_begin_update), then its instructions_file " + "and candidates_file. An active task must be resumed, not delegated twice. " + "Do not poll for readiness." + ) + return notice + + def _completed_candidates(self) -> set[str]: + return { + item["uid"] for path in (self.root / "tasks").glob("*.json") + for record in [_read_json(path)] if record is not None and record.get("receipt") is not None + for item in record["candidates"] + } + + def _check_current(self, task_id: str) -> None: + if self._active_id() != task_id or self._task(task_id)["status"] != "running": + raise AssistantClaimLost("assistant task was replaced; its edits cannot be completed") + + def _active_id(self) -> str | None: + record = _read_json(self.root / "active.json") + return None if record is None else record["task_id"] + + def _active(self) -> dict | None: + task_id = self._active_id() + return None if task_id is None else self._task(task_id) + + def _task(self, task_id: str) -> dict: + if not isinstance(task_id, str) or re.fullmatch(r"[0-9a-f]{32}", task_id) is None: + raise ValueError("task_id must be a task identifier returned by update begin") + record = _read_json(self.root / "tasks" / f"{task_id}.json") + if record is None: + raise FileNotFoundError(f"assistant task does not exist: {task_id}") + return record + + def _save_task(self, record: dict) -> None: + _write_json(self.root / "tasks" / f"{record['task_id']}.json", record) + + def _coordinator(self) -> GitUpdateQueueCoordinator: + coordinator = GitUpdateQueueCoordinator(self.sync_config) + # The old worker resumes leases held by its device. Give the native + # executor its own stable holder so it cannot share that live claim. + coordinator.device_id = hashlib.sha256( + f"{coordinator.device_id}:assistant".encode("utf-8") + ).hexdigest()[:32] + return coordinator + + @staticmethod + def _candidates(record: dict): + return tuple(parse_update_candidate_data(item) for item in record["candidates"]) + + def _claim(self, record: dict) -> ClaimedUpdateBatch: + data = record["claim"] + return ClaimedUpdateBatch( + parse_update_queue_lease_json(json.dumps({"schema_version": SCHEMA_VERSION, **data["lease"]})), + self._candidates(record), data["lease_commit"], + ) + + def _argv(self, *args: str) -> list[str]: + return ["rightmemory", "assistant", "--root", str(self.memory_root), *args] + + @contextmanager + def _locked(self): + _check_path(self.root / "state.lock") + _ensure_runtime_gitignore(self.memory_root / ".runtime") + self.root.mkdir(parents=True, exist_ok=True) + with (self.root / "state.lock").open("a+", encoding="utf-8") as handle: + lock_file(handle) + try: + yield + finally: + unlock_file(handle) + + +def _now() -> datetime: + return datetime.now(UTC) + + +def _time(value: str) -> datetime: + return datetime.fromisoformat(value).astimezone(UTC) + + +def _read_json(path: Path) -> dict | None: + _check_path(path) + try: + value = json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError: + return None + if not isinstance(value, dict): + raise ValueError(f"assistant state is not a JSON object: {path}") + return value + + +def _write_json(path: Path, value: dict) -> None: + _check_path(path) + path.parent.mkdir(parents=True, exist_ok=True) + temporary = path.with_name(f".{path.name}.{uuid.uuid4().hex}.tmp") + with temporary.open("w", encoding="utf-8") as handle: + json.dump(value, handle, ensure_ascii=False, indent=2) + handle.write("\n") + handle.flush() + os.fsync(handle.fileno()) + os.replace(temporary, path) + _fsync_directory(path.parent) + + +def _check_path(path: Path) -> None: + if path.is_symlink() or path.resolve() != path: + raise ValueError(f"assistant runtime path must not contain a symbolic link: {path}") diff --git a/rightmemory/assistant_cli.py b/rightmemory/assistant_cli.py new file mode 100644 index 0000000..a1cf151 --- /dev/null +++ b/rightmemory/assistant_cli.py @@ -0,0 +1,76 @@ +from __future__ import annotations + +import argparse +import json +from pathlib import Path + +from .config import default_memory_root +from .profiles import resolve_memory_root + + +class _Parser(argparse.ArgumentParser): + def error(self, message: str) -> None: + raise ValueError(message) + + +def main(argv: list[str], *, profile_name: str | None = None) -> int: + """JSON CLI over the same model-free core exposed by assistant MCP.""" + try: + parser = _Parser(prog="rightmemory assistant") + parser.add_argument("--root", type=Path) + commands = parser.add_subparsers(dest="command", required=True) + context = commands.add_parser("context", help="prepare native Retrieve file access") + context.add_argument("--session", required=True) + submit = commands.add_parser("submit", help="save candidate evidence without starting a worker") + submit.add_argument("--session", required=True) + submit.add_argument("--request-id", required=True) + submit.add_argument("--file", type=Path, required=True) + update = commands.add_parser("update") + updates = update.add_subparsers(dest="action", required=True) + begin = updates.add_parser("begin") + begin.add_argument("--retry", metavar="TASK") + complete = updates.add_parser("complete") + complete.add_argument("--task", required=True) + complete.add_argument("--output-file", type=Path) + status = updates.add_parser("status") + status.add_argument("--task") + sync = commands.add_parser("sync") + sync.add_argument("action", choices=("refresh", "pull", "push"), nargs="?", default="refresh") + commands.add_parser("validate", help="validate the bound root independently") + mcp = commands.add_parser("mcp", help="run the thin native-assistant MCP server") + mcp.add_argument("--transport", choices=("stdio",), default="stdio") + args = parser.parse_args(argv) + root = args.root + if root is None: + root = resolve_memory_root( + profile_name=profile_name, cwd=Path.cwd(), default_root=default_memory_root(), + ).memory_root + root = root.expanduser().resolve() + if args.command == "mcp": + from .assistant_mcp import mcp_main + return mcp_main(root, ["--transport", args.transport]) + if args.command == "validate": + from .validation import validate_memory_root + result = validate_memory_root(root).to_dict() + else: + from .assistant import AssistantService + service = AssistantService(root) + if args.command == "context": + result = service.context(args.session) + elif args.command == "submit": + result = service.submit(args.session, args.request_id, args.file.read_text(encoding="utf-8")) + elif args.command == "sync": + result = service.sync(args.action) + elif args.action == "begin": + result = service.begin_update(args.retry) + elif args.action == "complete": + output = "" if args.output_file is None else args.output_file.read_text(encoding="utf-8") + result = service.complete_update(args.task, output) + else: + result = service.update_status(args.task) + print(json.dumps(result, ensure_ascii=False)) + return 0 if result.get("ok", True) else 1 + except (ValueError, OSError, RuntimeError, UnicodeError) as exc: + print(json.dumps({"ok": False, "error": {"type": type(exc).__name__, "message": str(exc)}}, + ensure_ascii=False)) + return 1 diff --git a/rightmemory/assistant_context.py b/rightmemory/assistant_context.py new file mode 100644 index 0000000..28c1cfd --- /dev/null +++ b/rightmemory/assistant_context.py @@ -0,0 +1,105 @@ +from __future__ import annotations + +from collections.abc import Iterable +from dataclasses import asdict +from pathlib import Path +from tempfile import mkdtemp +from typing import Any + +from .config import load_sync_config +from .corrections import AGENT_CORRECTION_SOURCE_PATHS +from .graph import GraphManifest, build_graph_manifest +from .recent_submitted import ( + collect_recent_submitted_memory_readonly, + format_recent_submitted_block, +) +from .retrieve_context import current_memory_head +from .session import MemoryWriteLock, _ensure_runtime_gitignore +from .shared_view_files import pull_all_file_views +from .shared_view_models import load_connections +from .shared_view_package import valid_file_view_package +from .sync import SyncManager, SyncResult + + +def prepare_retrieve_context( + memory_root: Path, + *, + instructions: str | None = None, + excluded_candidate_uids: Iterable[str] = (), +) -> dict[str, Any]: + """Prepare native file access for an assistant-owned Retrieve task. + + The active root remains live. Only pending submissions and shared role + instructions are materialized for this invocation; no model, worker, or + delivery-history state is created. + """ + root = Path(memory_root).expanduser().resolve() + if not root.is_dir(): + raise FileNotFoundError(f"RightMemory memory root does not exist: {root}") + sync_config = load_sync_config(root) + try: + sync_result = SyncManager(sync_config).refresh_for_retrieve() + except Exception as exc: + sync_result = SyncResult( + "error", f"retrieve sync refresh failed: {type(exc).__name__}: {exc}" + ) + with MemoryWriteLock(root): + file_views = pull_all_file_views(root) + manifest = build_graph_manifest(root) + mf_imports = _mf_import_directories(root, manifest) + memory_commit = current_memory_head(root) + source_files = set(manifest.files) + source_files.update( + root / filename + for filename in AGENT_CORRECTION_SOURCE_PATHS.values() + if (root / filename).is_file() and not (root / filename).is_symlink() + ) + entries = collect_recent_submitted_memory_readonly( + root, excluded_candidate_uids=excluded_candidate_uids + ) + if instructions is None: + from .assistant_prompt import build_assistant_retrieve_instructions + + instructions = build_assistant_retrieve_instructions(root) + + runtime_root = root / ".runtime" + _ensure_runtime_gitignore(runtime_root) + output_root = runtime_root / "assistant" / "retrieve" + output_root.mkdir(parents=True, exist_ok=True) + prepared_root = Path(mkdtemp(prefix="retrieve-", dir=output_root)) + pending_file = prepared_root / "PENDING.md" + pending_file.write_text(format_recent_submitted_block(entries), encoding="utf-8") + instructions_file = prepared_root / "RETRIEVE.md" + instructions_file.write_text(instructions, encoding="utf-8") + return { + "memory_root": str(root), + "memory_commit": memory_commit, + "live_files": True, + "pending_file": str(pending_file), + "instructions_file": str(instructions_file), + "source_files": [str(path) for path in sorted(source_files)], + "mf_imports": mf_imports, + "pending_count": len(entries), + "sync": asdict(sync_result), + "file_views": [asdict(result) for result in file_views], + } + + +def _mf_import_directories(root: Path, manifest: GraphManifest) -> dict[str, str]: + connections = load_connections(root) + imports: dict[str, str] = {} + for heading_id, heading in sorted(manifest.headings.items()): + if heading.anchor_kind != "MF#": + continue + package_root = root / ".runtime" / "shared_views" / "imports" / heading_id + connection = connections.get(heading_id) + view_id = ( + connection.target.view_id + if connection is not None and connection.target.view_id + else heading_id + ) + if valid_file_view_package( + package_root, expected_view_id=view_id, namespace_id=heading_id + ): + imports[heading_id] = str(package_root / "dist") + return imports diff --git a/rightmemory/assistant_mcp.py b/rightmemory/assistant_mcp.py new file mode 100644 index 0000000..656e41f --- /dev/null +++ b/rightmemory/assistant_mcp.py @@ -0,0 +1,125 @@ +from __future__ import annotations + +import argparse +import json +from collections.abc import Callable +from pathlib import Path +from typing import Any, Literal, Protocol + +from mcp.server import MCPServer +from mcp.types import CallToolResult, TextContent + +from .validation import validate_memory_root + + +class AssistantBackend(Protocol): + def context(self, session_id: str) -> dict[str, Any]: ... + + def submit( + self, session_id: str, request_id: str, evidence: str + ) -> dict[str, Any]: ... + + def begin_update(self, retry_task: str | None = None) -> dict[str, Any]: ... + + def complete_update(self, task_id: str, output: str = "") -> dict[str, Any]: ... + + def update_status(self, task_id: str | None = None) -> dict[str, Any]: ... + + def sync(self, action: str = "refresh") -> dict[str, Any]: ... + + +def create_mcp_server( + memory_root: Path, *, backend: AssistantBackend | None = None +) -> MCPServer: + """Expose the assistant core without importing an internal role runtime.""" + root = Path(memory_root).expanduser().resolve() + if backend is None: + from .assistant import AssistantService + + backend = AssistantService(root) + selected = backend + server = MCPServer("RightMemory Assistant", log_level="WARNING") + + @server.tool( + name="rightmemory_assistant_context", + description="Prepare source paths and shared instructions for a native Retrieve subagent.", + structured_output=False, + ) + def context(session_id: str) -> CallToolResult: + return _invoke(lambda: selected.context(session_id)) + + @server.tool( + name="rightmemory_assistant_submit", + description="Save candidate evidence with a stable retry id; return pending and batch notices.", + structured_output=False, + ) + def submit(session_id: str, request_id: str, evidence: str) -> CallToolResult: + return _invoke(lambda: selected.submit(session_id, request_id, evidence)) + + @server.tool( + name="rightmemory_assistant_begin_update", + description="Prepare a ready batch and isolated worktree for a native Update subagent.", + structured_output=False, + ) + def begin_update(retry_task: str | None = None) -> CallToolResult: + return _invoke(lambda: selected.begin_update(retry_task)) + + @server.tool( + name="rightmemory_assistant_complete_update", + description="Validate and finalize the specified candidate task; retries use the same task id.", + structured_output=False, + ) + def complete_update(task_id: str, output: str = "") -> CallToolResult: + return _invoke(lambda: selected.complete_update(task_id, output)) + + @server.tool( + name="rightmemory_assistant_update_status", + description="Inspect native Update execution and recovery state.", + structured_output=False, + ) + def update_status(task_id: str | None = None) -> CallToolResult: + return _invoke(lambda: selected.update_status(task_id)) + + @server.tool( + name="rightmemory_assistant_sync", + description="Run the configured deterministic sync operation and report its outcome.", + structured_output=False, + ) + def sync(action: Literal["refresh", "pull", "push"] = "refresh") -> CallToolResult: + return _invoke(lambda: selected.sync(action)) + + @server.tool( + name="rightmemory_assistant_validate", + description="Independently validate the active memory root bound to this server.", + structured_output=False, + ) + def validate() -> CallToolResult: + return _invoke(lambda: validate_memory_root(root).to_dict()) + + return server + + +def _invoke(operation: Callable[[], dict[str, Any]]) -> CallToolResult: + try: + payload = operation() + except (ValueError, RuntimeError, OSError) as exc: + payload = { + "ok": False, + "error": {"type": type(exc).__name__, "message": str(exc)}, + } + return CallToolResult( + content=[TextContent(type="text", text=json.dumps(payload, ensure_ascii=False))], + structured_content=payload, + is_error=payload.get("ok") is False, + ) + + +def mcp_main(memory_root: Path, argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="rightmemory assistant mcp", + description="Serve the deterministic assistant core over MCP stdio.", + ) + parser.add_argument("--transport", choices=["stdio"], default="stdio") + args = parser.parse_args([] if argv is None else argv) + create_mcp_server(memory_root).run(transport=args.transport) + return 0 diff --git a/rightmemory/assistant_prompt.py b/rightmemory/assistant_prompt.py new file mode 100644 index 0000000..65d44ad --- /dev/null +++ b/rightmemory/assistant_prompt.py @@ -0,0 +1,48 @@ +from __future__ import annotations + +from pathlib import Path + +from .prompt import _read_prompt_file, build_cli_agent_instructions +from .reference import read_reference + + +def build_assistant_retrieve_instructions(memory_root: Path) -> str: + """Build the native assistant role without loading a model or memory state.""" + return f"""You are the RightMemory Retrieve subagent for this assistant. + +Use the assistant's native file and search tools. The active memory root is {memory_root}. +The context result supplies source_files, mf_imports, and pending_file; inspect those +real sources as needed. The caller supplies the retrieval need. + +RightMemory schema: +{read_reference('schema')} + +Role instructions: +{_read_prompt_file('prompts/assistant-retrieve.md')} +""" + + +def build_assistant_update_instructions( + worktree: Path, task_id: str, *, active_root: Path +) -> str: + """Reuse Update judgment and native Git work, then return to program finalization.""" + canonical = build_cli_agent_instructions(worktree, "update") + return f"""{canonical} + +Assistant execution: +- This is isolated task {task_id}. The writable candidate worktree is {worktree}. +- The active root for assistant control commands is {active_root}. Keep that binding + when running commands from the candidate worktree. +- Use native file and Git tools for the canonical Update role. Validate with + `rightmemory validate --root ` before committing the allowed changes. +- After the worktree is clean, call the task's supplied complete_command, or the + matching assistant MCP completion tool. The command is + `rightmemory assistant --root update complete --task `. + Complete an unchanged task too so its batch can be recorded. +- A worktree commit is candidate work. Only the program's completion receipt + confirms application to the active root; report its application and sync outcome. +- If completion is interrupted or its reply is lost, inspect the same task's status + and retry its completion as directed. Preserve the worktree and task identity. + A prepared result awaiting publication needs the same completion retry, not + another semantic edit of its candidate. +""" diff --git a/rightmemory/assistant_write.py b/rightmemory/assistant_write.py new file mode 100644 index 0000000..619d13c --- /dev/null +++ b/rightmemory/assistant_write.py @@ -0,0 +1,369 @@ +from __future__ import annotations + +import hashlib +import json +import os +import re +import uuid +from collections.abc import Callable, Iterable, Iterator +from contextlib import contextmanager +from dataclasses import dataclass, replace +from pathlib import Path + +from .isolated_write import ( + IsolatedWriteResult, + IsolatedWriteSupervisor, + MainMemoryDirtyError, + MemoryVersionConflictError, +) +from .platform import lock_file, unlock_file +from .semantic_operation import FINAL_PHASES, SemanticOperationStore +from .session import MemoryWriteLock, _ensure_durable_directory, _ensure_runtime_gitignore, _fsync_directory +from .update_queue import UpdateCandidate, parse_update_candidate_data, update_candidate_data +from .update_record import UpdateRecord, UpdateRecordStore + + +_OID = re.compile(r"(?:[0-9a-f]{40}|[0-9a-f]{64})") + + +class AssistantStaleVersionError(MemoryVersionConflictError): + """The external task must be reconsidered against newer memory.""" + + +@dataclass(frozen=True) +class AssistantWriteTask: + task_id: str + batch_id: str + worktree: Path + base_commit: str + branch: str + candidates: tuple[UpdateCandidate, ...] + operation_id: str + source_commit: str | None = None + output: str | None = None + external_finalizer: str | None = None + + +class AssistantWriteManager: + """Keep native-edit worktrees until a fenced, validated completion is durable.""" + + def __init__(self, memory_root: Path): + self.memory_root = Path(memory_root).resolve() + self.state_root = self.memory_root / ".runtime" / "assistant-write" + self.supervisor = IsolatedWriteSupervisor(self.memory_root, "update") + + def prepare( + self, + task_id: str, + batch_id: str, + candidates: Iterable[UpdateCandidate], + *, + check_current: Callable[[], None] | None = None, + ) -> AssistantWriteTask: + evidence = UpdateRecord.from_candidates(candidates) + if evidence.operation_id != batch_id: + raise ValueError("assistant batch id does not match its candidate evidence") + self.supervisor._ensure_repo_root() + with self._task_locked(task_id): + task = self._read(task_id) + if task is not None: + if task.batch_id != batch_id or task.candidates != evidence.candidates: + raise ValueError("assistant task already belongs to different candidate evidence") + if check_current is not None: + check_current() + receipt = SemanticOperationStore(self.memory_root).read(task.operation_id) + if receipt is not None and receipt.phase in FINAL_PHASES: + return task + self._ensure_worktree(task) + return task + store = SemanticOperationStore(self.memory_root) + with store.execution_locked(), MemoryWriteLock(self.memory_root): + if check_current is not None: + check_current() + dirty = self.supervisor._dirty_memory_files() + if dirty: + raise MainMemoryDirtyError(dirty) + task = self._task( + task_id, + batch_id, + self.supervisor._git_stdout(self.memory_root, "rev-parse", "HEAD"), + evidence.candidates, + ) + self._write(task) + self._ensure_worktree(task) + return task + + def read(self, task_id: str) -> AssistantWriteTask: + task = self._read(task_id) + if task is None: + raise FileNotFoundError(f"assistant write task does not exist: {task_id}") + return task + + def complete( + self, + task_id: str, + *, + output: str = "", + check_current: Callable[[], None] | None = None, + external_finalizer: str | None = None, + ) -> IsolatedWriteResult: + if not isinstance(output, str): + raise ValueError("assistant output must be a string") + finalizer = external_finalizer if external_finalizer is not None else f"assistant:{task_id}" + if not isinstance(finalizer, str) or not finalizer.strip(): + raise ValueError("external_finalizer must be a non-empty string") + with self._task_locked(task_id): + task = self.read(task_id) + if task.source_commit is not None: + if task.external_finalizer != finalizer: + raise ValueError("assistant completion belongs to a different finalizer") + if output and output != task.output: + raise ValueError("assistant completion already has a different output") + else: + self._verify_worktree(task) + if self.supervisor._git_stdout(task.worktree, "status", "--porcelain", "--untracked-files=all"): + raise RuntimeError("assistant worktree has uncommitted changes") + source_commit = self.supervisor._git_stdout(task.worktree, "rev-parse", "HEAD") + self._source_commits(task, source_commit) + task = replace(task, source_commit=source_commit, output=output, external_finalizer=finalizer) + self._write(task) + + def import_commits(worktree: Path) -> str: + current = self.supervisor._git_stdout(worktree, "rev-parse", "HEAD") + changed = self.supervisor._semantic_changes_between(task.base_commit, current) + if changed: + raise AssistantStaleVersionError("memory changed since assistant task began: " + ", ".join(changed)) + for commit in self._source_commits(task, task.source_commit): + self.supervisor._run_git(worktree, "cherry-pick", "--allow-empty", commit) + # The no-change path must also pass complete graph validation. + self.supervisor._validate_candidate(worktree, []) + return task.output or "" + + def retain_evidence(worktree: Path, *_args: object) -> tuple[str, ...]: + records = UpdateRecordStore(worktree) + if records.root.is_symlink(): + raise ValueError("update records must not be a symlink") + evidence = UpdateRecord.from_candidates(task.candidates) + existing = records.read(task.batch_id) + if existing is not None: + if existing != evidence: + raise ValueError("assistant batch already has different retained evidence") + return () + path = records.write(evidence) + return (path.relative_to(worktree).as_posix(),) + + try: + result = self.supervisor.run( + import_commits, + operation_id=task.operation_id, + operation_input={ + "kind": "assistant-update", + "task_id": task.task_id, + "batch_id": task.batch_id, + "base_commit": task.base_commit, + "candidates": [update_candidate_data(item) for item in task.candidates], + }, + prepare_managed_artifacts=retain_evidence, + external_finalizer=finalizer, + check_current=check_current, + ) + if external_finalizer is None: + result = self.supervisor.publish_external( + task.operation_id, + external_finalizer=finalizer, + check_current=check_current, + ) + return result + except Exception as exc: + receipt = SemanticOperationStore(self.memory_root).read(task.operation_id) + if receipt is None or receipt.phase == "running": + # Rejected edits may be corrected in the same native worktree. + self._write(replace(task, source_commit=None, output=None, external_finalizer=None)) + if isinstance(exc, MemoryVersionConflictError) and not isinstance(exc, AssistantStaleVersionError): + raise AssistantStaleVersionError(str(exc)) from exc + raise + + def complete_external( + self, + task_id: str, + *, + external_finalizer: str, + landed_commit: str, + ) -> IsolatedWriteResult: + with self._task_locked(task_id): + task = self.read(task_id) + if task.external_finalizer != external_finalizer: + raise ValueError("assistant completion belongs to a different finalizer") + return self.supervisor.complete_external( + task.operation_id, + external_finalizer=external_finalizer, + landed_commit=landed_commit, + ) + + def cleanup(self, task_id: str) -> None: + """Remove a settled workspace, retaining its task identity and receipt.""" + with self._task_locked(task_id): + task = self.read(task_id) + receipt = SemanticOperationStore(self.memory_root).read(task.operation_id) + if receipt is None or receipt.phase not in FINAL_PHASES: + raise RuntimeError("assistant worktree cannot be removed before completion") + if not task.worktree.exists(): + return + self._verify_worktree(task) + if self.supervisor._git_stdout(task.worktree, "status", "--porcelain", "--untracked-files=all"): + raise RuntimeError("assistant worktree has new uncommitted changes; retained for review") + if self.supervisor._git_stdout(task.worktree, "rev-parse", "HEAD") != task.source_commit: + raise RuntimeError("assistant worktree changed after completion; retained for review") + self.supervisor._run_git(self.memory_root, "worktree", "remove", str(task.worktree)) + self.supervisor._run_git(self.memory_root, "branch", "-D", task.branch) + + def _source_commits(self, task: AssistantWriteTask, source_commit: str | None) -> list[str]: + if source_commit is None or _OID.fullmatch(source_commit) is None: + raise ValueError("assistant source must be a full commit id") + if not self.supervisor._is_ancestor(task.base_commit, source_commit): + raise RuntimeError("assistant worktree no longer descends from its prepared base") + commits = self.supervisor._git_stdout( + self.memory_root, "rev-list", "--reverse", f"{task.base_commit}..{source_commit}" + ).splitlines() + parent = task.base_commit + for commit in commits: + parents = self.supervisor._git_stdout( + self.memory_root, "rev-list", "--parents", "-1", commit + ).split() + if parents != [commit, parent]: + raise RuntimeError("assistant worktree must have linear commits from its prepared base") + self.supervisor._validate_commits(self.memory_root, [commit]) + parent = commit + return commits + + def _ensure_worktree(self, task: AssistantWriteTask) -> None: + self._check_runtime_paths(task.worktree) + if (task.worktree / ".git").exists(): + self._verify_worktree(task) + return + if task.worktree.exists() and any(task.worktree.iterdir()): + raise RuntimeError("assistant worktree path contains unregistered files; left unchanged") + self.supervisor._ensure_runtime_ignored(task.worktree) + branch_exists = self.supervisor._run_git( + self.memory_root, "show-ref", "--verify", "--quiet", f"refs/heads/{task.branch}", check=False + ).returncode == 0 + args = ["worktree", "add"] + if branch_exists: + args.extend([str(task.worktree), task.branch]) + else: + args.extend(["-b", task.branch, str(task.worktree), task.base_commit]) + self.supervisor._run_git(self.memory_root, *args) + self.supervisor._seed_untracked_publish_artifacts(task.worktree) + self._verify_worktree(task) + + def _verify_worktree(self, task: AssistantWriteTask) -> None: + self._check_runtime_paths(task.worktree) + if not task.worktree.is_dir(): + raise FileNotFoundError(f"assistant worktree is missing: {task.worktree}") + top = Path(self.supervisor._git_stdout(task.worktree, "rev-parse", "--show-toplevel")).resolve() + common = self.supervisor._git_stdout(task.worktree, "rev-parse", "--git-common-dir") + active_common = self.supervisor._git_stdout(self.memory_root, "rev-parse", "--git-common-dir") + branch = self.supervisor._git_stdout(task.worktree, "symbolic-ref", "--short", "HEAD") + if ( + top != task.worktree + or (task.worktree / common).resolve() != (self.memory_root / active_common).resolve() + or branch != task.branch + ): + raise RuntimeError("assistant worktree identity does not match its prepared task") + + def _task( + self, task_id: str, batch_id: str, base_commit: str, candidates: tuple[UpdateCandidate, ...], + *, source_commit: str | None = None, output: str | None = None, + external_finalizer: str | None = None, + ) -> AssistantWriteTask: + digest = _task_digest(task_id) + return AssistantWriteTask( + task_id=task_id, batch_id=batch_id, base_commit=base_commit, candidates=candidates, + worktree=self.memory_root / ".runtime" / "worktrees" / f"assistant-{digest}", + branch=f"rightmemory-assistant-{digest}", operation_id=f"assistant-update-{digest}", + source_commit=source_commit, output=output, external_finalizer=external_finalizer, + ) + + def _record_path(self, task_id: str) -> Path: + return self.state_root / f"{_task_digest(task_id)}.json" + + def _read(self, task_id: str) -> AssistantWriteTask | None: + path = self._record_path(task_id) + self._check_runtime_paths(path) + if not path.exists(): + return None + if not path.is_file(): + raise ValueError("assistant task record must be a regular file") + data = json.loads(path.read_text(encoding="utf-8")) + keys = {"version", "task_id", "batch_id", "base_commit", "candidates", "source_commit", "output", "external_finalizer"} + if not isinstance(data, dict) or set(data) != keys or type(data["version"]) is not int or data["version"] != 1: + raise ValueError("invalid assistant task record") + if data["task_id"] != task_id or not isinstance(data["base_commit"], str) or not _OID.fullmatch(data["base_commit"]): + raise ValueError("invalid assistant task identity or base commit") + if not isinstance(data["candidates"], list): + raise ValueError("assistant task candidates must be a list") + evidence = UpdateRecord.from_candidates(parse_update_candidate_data(item) for item in data["candidates"]) + if evidence.operation_id != data["batch_id"]: + raise ValueError("assistant task batch does not match its candidate evidence") + source = data["source_commit"] + if source is None: + if data["output"] is not None or data["external_finalizer"] is not None: + raise ValueError("unsubmitted assistant task has completion fields") + elif ( + not isinstance(source, str) or not _OID.fullmatch(source) + or not isinstance(data["output"], str) + or not isinstance(data["external_finalizer"], str) or not data["external_finalizer"].strip() + ): + raise ValueError("invalid assistant task completion") + return self._task( + task_id, data["batch_id"], data["base_commit"], evidence.candidates, + source_commit=source, output=data["output"], external_finalizer=data["external_finalizer"], + ) + + def _write(self, task: AssistantWriteTask) -> None: + path = self._record_path(task.task_id) + self._check_runtime_paths(path) + data = { + "version": 1, "task_id": task.task_id, "batch_id": task.batch_id, + "base_commit": task.base_commit, + "candidates": [update_candidate_data(item) for item in task.candidates], + "source_commit": task.source_commit, "output": task.output, + "external_finalizer": task.external_finalizer, + } + temporary = path.with_name(f".{path.name}.{uuid.uuid4().hex}.tmp") + try: + with temporary.open("x", encoding="utf-8") as handle: + handle.write(json.dumps(data, ensure_ascii=False, sort_keys=True, allow_nan=False) + "\n") + handle.flush() + os.fsync(handle.fileno()) + os.replace(temporary, path) + _fsync_directory(path.parent) + finally: + temporary.unlink(missing_ok=True) + + @contextmanager + def _task_locked(self, task_id: str) -> Iterator[None]: + path = self.state_root / f"{_task_digest(task_id)}.lock" + self._check_runtime_paths(path) + _ensure_runtime_gitignore(self.memory_root / ".runtime") + _ensure_durable_directory(self.state_root) + with path.open("a+b") as handle: + lock_file(handle) + try: + yield + finally: + unlock_file(handle) + + def _check_runtime_paths(self, path: Path) -> None: + relative = path.relative_to(self.memory_root) + current = self.memory_root + for part in relative.parts: + current /= part + if current.is_symlink(): + raise ValueError(f"assistant runtime path must not be a symlink: {current}") + + +def _task_digest(task_id: str) -> str: + if not isinstance(task_id, str) or not task_id.strip() or len(task_id) > 200 or any(c in task_id for c in "\0\r\n"): + raise ValueError("assistant task id must be a non-empty single-line string of at most 200 characters") + return hashlib.sha256(task_id.encode("utf-8")).hexdigest() diff --git a/rightmemory/async_update.py b/rightmemory/async_update.py index 3a060b9..e573605 100644 --- a/rightmemory/async_update.py +++ b/rightmemory/async_update.py @@ -76,6 +76,7 @@ class AsyncUpdateState: current_batch: list[AsyncUpdateJob] = field(default_factory=list) pending: list[AsyncUpdateJob] = field(default_factory=list) next_id: int = 1 + executor: str = "worker" @dataclass(frozen=True) @@ -91,6 +92,7 @@ class AsyncBatchReservation: operation_id: str created_at: str participants: list[AsyncUpdateSessionBatch] + executor: str = "worker" @dataclass(frozen=True) @@ -127,9 +129,12 @@ class _WorkerSnapshot: class AsyncUpdateStore: - def __init__(self, memory_root: Path, role: str): + def __init__(self, memory_root: Path, role: str, *, executor: str = "worker"): + if executor not in {"worker", "assistant"}: + raise ValueError("unknown update executor") self.memory_root = memory_root self.role = role + self.executor = executor self.root = memory_root / ".runtime" / "async" / role self.worker_root = self.root / "_worker" self.reservations_root = self.root / "_batches" @@ -210,7 +215,7 @@ def retry_manual_recovery(self) -> AsyncUpdateRetryResult: except (OSError, json.JSONDecodeError, ValueError): skipped_sessions += 1 continue - if state.status != STATUS_MANUAL_RECOVERY: + if state.executor != self.executor or state.status != STATUS_MANUAL_RECOVERY: continue retry_count = len(state.current_batch) + len(state.pending) if not retry_count: @@ -277,6 +282,13 @@ def submit( with self._locked(session_id): current = self._read_checked_locked(session_id) current = self._reconcile_session_outbox_locked(current) + if current.executor != self.executor and (current.current_batch or current.pending): + raise ValueError("update session is already owned by a different executor") + current = replace(current, executor=self.executor) + if self.executor == "assistant": + # Preserve executor ownership even if an outbox-first submit + # is interrupted before the complete session state is saved. + self._write(session_id, current) matching = [ job for job in [*current.current_batch, *current.pending] @@ -296,7 +308,7 @@ def submit( UpdateQueueStore(self.memory_root).write_outbox( self._candidate_from_job(session_id, job) ) - worker_pid = self._active_worker_pid() + worker_pid = self._active_worker_pid() if self.executor == "worker" else None current = replace( current, accepted_candidate_uids=[ @@ -307,10 +319,35 @@ def submit( state = self._enqueue_locked(current, job, now=now, worker_pid=worker_pid) self._write(session_id, state) - if state.status != STATUS_MANUAL_RECOVERY: + if self.executor == "worker" and state.status != STATUS_MANUAL_RECOVERY: self._start_worker_if_needed(session_id) return state + def ready_batch( + self, *, trigger_candidates: int, target_batch_candidates: int, max_wait_seconds: int, + ) -> tuple[list[AsyncUpdateSessionBatch] | None, datetime | None]: + """Inspect the existing batching policy without starting an executor.""" + return self._next_batch(trigger_candidates, target_batch_candidates, max_wait_seconds) + + def reserve_batch(self, batch: list[AsyncUpdateSessionBatch]) -> AsyncBatchReservation | None: + return self._reserve_cross_session_batch(batch, _batch_session_id(batch)) + + def start_reserved_batch(self, operation_id: str) -> tuple[UpdateCandidate, ...]: + reservation = self._read_reservation(operation_id) + if reservation is None or reservation.executor != self.executor: + raise RuntimeError("update batch does not belong to this executor") + self._start_cross_session_batch(reservation.participants, operation_id) + return self.operation_candidates(operation_id) + + def finish_reserved_batch(self, operation_id: str, result: str) -> None: + reservation = self._read_reservation(operation_id) + if reservation is None: + return + if reservation.executor != self.executor: + raise RuntimeError("update batch does not belong to this executor") + self._finish_cross_session_batch(reservation.participants, operation_id, result) + self._clear_reservation_if_acknowledged(operation_id) + def acknowledge_synchronized( self, candidate_uids: set[str] | frozenset[str], @@ -759,7 +796,7 @@ def _next_batch( continue with self._locked(session_id): state = self._read_checked_locked(session_id) - if state.role != self.role: + if state.role != self.role or state.executor != self.executor: continue if state.current_batch: operation_id = state.current_operation_id @@ -880,6 +917,8 @@ def _next_reserved_batch( session_id = sorted(overlap)[0] raise RuntimeError(f"async session belongs to multiple reserved batches: {session_id}") reserved_sessions.update(participant_ids) + if reservation.executor != self.executor: + continue if not blocked and not retry_deadlines: ready.append(reservation) elif not blocked and retry_deadlines: @@ -937,6 +976,8 @@ def _reserve_cross_session_batch( path = self._reservation_path(operation_id) if path.exists(): existing = self._read_reservation_path(path) + if existing.executor != self.executor: + raise RuntimeError("async batch belongs to a different executor") if existing.participants != participants: raise RuntimeError("async operation id already belongs to a different reserved batch") return existing @@ -952,6 +993,8 @@ def _reserve_cross_session_batch( for item in participants: with self._locked(item.session_id): state = self._read_raw(item.session_id) + if state.executor != self.executor: + raise RuntimeError(f"async reserved batch has a different executor: {item.session_id}") if fresh_selection and any( UpdateQueueStore(self.memory_root).publication_state(job.candidate_uid) == "attempted" @@ -974,6 +1017,7 @@ def _reserve_cross_session_batch( operation_id=operation_id, created_at=_now(), participants=participants, + executor=self.executor, ) self._write_reservation_locked(path, reservation) return reservation @@ -1064,7 +1108,10 @@ def _read_reservation_path(self, path: Path) -> AsyncBatchReservation: raise ValueError("async batch reservation does not match its operation id") if path.name != self._reservation_path(operation_id).name: raise ValueError("async batch reservation filename does not match its operation id") - return AsyncBatchReservation(operation_id, created_at, participants) + executor = data.get("executor", "worker") + if executor not in {"worker", "assistant"}: + raise ValueError("unknown async batch executor") + return AsyncBatchReservation(operation_id, created_at, participants, executor) def _write_reservation_locked(self, path: Path, reservation: AsyncBatchReservation) -> None: content = json.dumps( @@ -1072,6 +1119,7 @@ def _write_reservation_locked(self, path: Path, reservation: AsyncBatchReservati "version": 1, "operation_id": reservation.operation_id, "created_at": reservation.created_at, + "executor": reservation.executor, "participants": [ { "session_id": item.session_id, @@ -1158,7 +1206,7 @@ def _start_cross_session_batch( finished_at=None, result=None, error=None, - pid=os.getpid(), + pid=os.getpid() if state.executor == "worker" else None, current_operation_id=operation_id, ) self._write(item.session_id, next_state) @@ -1188,7 +1236,7 @@ def _start_cross_session_batch( next_flush_at=state.next_flush_at if pending else None, result=None, error=None, - pid=os.getpid(), + pid=os.getpid() if state.executor == "worker" else None, current_operation_id=operation_id, ) self._write(item.session_id, next_state) @@ -1225,7 +1273,7 @@ def _finish_cross_session_batch( def _finished_state(self, state: AsyncUpdateState, operation_id: str, result: str) -> AsyncUpdateState: common = { - "pid": os.getpid(), + "pid": os.getpid() if state.executor == "worker" else None, "current_batch": [], "current_operation_id": None, "last_operation_id": operation_id, @@ -1531,6 +1579,10 @@ def _session_state_paths(self) -> list[Path]: def _read_checked_locked(self, session_id: str) -> AsyncUpdateState: state = self._read_raw(session_id) + # Native child tasks outlive individual CLI processes. Their durable + # claim is settled by AssistantService, never by worker PID inference. + if state.executor == "assistant" or state.executor != self.executor: + return state if state.current_batch and state.current_operation_id is not None: terminal_output = self._terminal_operation_output(state.current_operation_id) if terminal_output is not None: @@ -1573,6 +1625,7 @@ def _enqueue_locked( last_operation_id=state.last_operation_id, accepted_candidate_uids=state.accepted_candidate_uids, next_id=next_id, + executor=state.executor, ) if state.status in {"failed", STATUS_MANUAL_RECOVERY}: return replace( @@ -1608,6 +1661,7 @@ def _enqueue_locked( last_operation_id=state.last_operation_id, accepted_candidate_uids=state.accepted_candidate_uids, next_id=next_id, + executor=state.executor, ) def _reconcile_session_outbox_locked( @@ -1773,7 +1827,7 @@ def _state_path(self, session_id: str) -> Path: def _read_raw(self, session_id: str) -> AsyncUpdateState: state_path = self._state_path(session_id) if not state_path.exists(): - return AsyncUpdateState(status="idle", session_id=session_id, role=self.role) + return AsyncUpdateState(status="idle", session_id=session_id, role=self.role, executor=self.executor) data = json.loads(state_path.read_text(encoding="utf-8")) return _state_from_json(data) @@ -1815,6 +1869,9 @@ def _state_from_json(data: object) -> AsyncUpdateState: status = _required_state_str(data, "status") session_id = _required_state_str(data, "session_id") role = _required_state_str(data, "role") + executor = data.get("executor", "worker") + if executor not in {"worker", "assistant"}: + raise ValueError("unknown async update executor") next_id = data.get("next_id") if isinstance(next_id, bool) or not isinstance(next_id, int) or next_id < 1: raise ValueError("async update state must contain positive integer field: next_id") @@ -1862,6 +1919,7 @@ def _state_from_json(data: object) -> AsyncUpdateState: current_batch=current_batch, pending=pending, next_id=next_id, + executor=executor, ) diff --git a/rightmemory/cli.py b/rightmemory/cli.py index 258b127..f8e2da4 100644 --- a/rightmemory/cli.py +++ b/rightmemory/cli.py @@ -82,7 +82,7 @@ from .share_results import format_share_operation_result from .shares import approve_share, create_share, join_share, list_shares, publish_share, revise_share, share_status from .status import collect_status, format_status_dashboard -from .sync import LEGACY_ROOT_REFERENCE_PATHS, REQUIRED_ROOT_DOCUMENTS, SyncManager +from .sync import SyncManager from .tools import MemoryTools from .update_queue_git import ( QUEUE_FINALIZER, @@ -1412,33 +1412,11 @@ def _validate_main(argv: list[str], profile_name: str | None) -> int: ) root = active.memory_root.expanduser().resolve() - document_errors = [ - f"required root document is not a regular file: {name}" - for name in REQUIRED_ROOT_DOCUMENTS - if (root / name).is_symlink() or not (root / name).is_file() - ] - document_errors.extend( - f"legacy package reference must be removed from the root: {name}" - for name in sorted(LEGACY_ROOT_REFERENCE_PATHS) - if os.path.lexists(root / name) - ) - corrections_path = root / "corrections.md" - if corrections_path.is_symlink() or ( - corrections_path.exists() and not corrections_path.is_file() - ): - document_errors.append( - "optional root document is not a regular file: corrections.md" - ) - if document_errors: - print( - "validation failed:\n" - + "\n".join(f"- {error}" for error in document_errors) - ) - return 1 + from .validation import validate_memory_root - report = MemoryTools(root, role="sync-reconciler").validate_memory() - print(report) - return 0 if report.startswith("validation passed:") else 1 + result = validate_memory_root(root) + print(result.report) + return 0 if result.ok else 1 def _prune_main(argv: list[str], memory_root: Path) -> int: diff --git a/rightmemory/entrypoint.py b/rightmemory/entrypoint.py index 67c3be9..a805a2e 100644 --- a/rightmemory/entrypoint.py +++ b/rightmemory/entrypoint.py @@ -4,15 +4,31 @@ import sys from pathlib import Path -from . import cli from .config import default_memory_root -from .guidance import GUIDANCE_INBOX_PATH, submit_guidance, validate_guidance_inbox +from .guidance import submit_guidance from .profiles import ProfileError, resolve_memory_root def main(argv: list[str] | None = None) -> int: args = list(sys.argv[1:] if argv is None else argv) profile_name, remaining = _parse_global_args(args) + if remaining[:1] == ["assistant"]: + from .assistant_cli import main as assistant_main + + return assistant_main(remaining[1:], profile_name=profile_name) + if remaining[:1] == ["validate"]: + try: + parser = argparse.ArgumentParser(prog="rightmemory validate") + parser.add_argument("--root", type=Path) + parser.parse_args(remaining[1:]) + from .validation import validate_memory_root + + result = validate_memory_root(_validation_root(profile_name, remaining[1:])) + print(result.report) + return 0 if result.ok else 1 + except (ValueError, ProfileError, OSError, RuntimeError, UnicodeError) as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 if remaining[:1] == ["embedding-service"]: try: from .embedding_service import main as embedding_service_main @@ -53,21 +69,9 @@ def main(argv: list[str] | None = None) -> int: except (ValueError, ProfileError, FileNotFoundError, RuntimeError) as exc: print(f"error: {exc}", file=sys.stderr) return 1 - result = cli.main(args) - if result != 0 or remaining[:1] != ["validate"]: - return result - try: - root = _validation_root(profile_name, remaining[1:]) - errors = _guidance_validation_errors(root) - except (ValueError, ProfileError, FileNotFoundError, OSError, UnicodeError) as exc: - print(f"error: {exc}", file=sys.stderr) - return 1 - if errors: - print("guidance validation failed:", file=sys.stderr) - for error in errors: - print(f"- {error}", file=sys.stderr) - return 1 - return 0 + from . import cli + + return cli.main(args) def _parse_global_args(argv: list[str]) -> tuple[str | None, list[str]]: @@ -109,12 +113,9 @@ def _validation_root(profile_name: str | None, argv: list[str]) -> Path: def _guidance_validation_errors(memory_root: Path) -> list[str]: - path = memory_root / GUIDANCE_INBOX_PATH - if not path.exists() and not path.is_symlink(): - return [] - if path.is_symlink() or not path.is_file(): - return [f"{GUIDANCE_INBOX_PATH} must be a regular file"] - return validate_guidance_inbox(path.read_text(encoding="utf-8")) + from .validation import guidance_validation_errors + + return guidance_validation_errors(memory_root) if __name__ == "__main__": diff --git a/rightmemory/isolated_write.py b/rightmemory/isolated_write.py index 95299e0..f4f256e 100644 --- a/rightmemory/isolated_write.py +++ b/rightmemory/isolated_write.py @@ -102,6 +102,10 @@ def __init__(self, paths: list[str]): super().__init__(f"main memory files have uncommitted changes: {detail}") +class MemoryVersionConflictError(RuntimeError): + """The validated operation can no longer publish against its original memory state.""" + + @dataclass(frozen=True) class IsolatedWriteResult: output: Any @@ -176,6 +180,26 @@ def complete_external( self._delete_operation_ref(operation_id) return self._result_from_operation(completed) + def publish_external( + self, + operation_id: str, + *, + external_finalizer: str, + check_current: Callable[[], None] | None = None, + ) -> IsolatedWriteResult: + """Publish a prepared local outcome under its owner's current fence.""" + self._ensure_repo_root() + store = SemanticOperationStore(self.memory_root) + with store.execution_locked(): + operation = store.read(operation_id) + if operation is None: + raise FileNotFoundError(f"semantic operation does not exist: {operation_id}") + self._require_external_finalizer(operation, external_finalizer) + if operation.phase in FINAL_PHASES: + return self._result_from_operation(operation) + operation = store.claim_prepared(operation_id) + return self._resume_prepared_operation(store, operation, check_current=check_current) + def restart_external( self, operation_id: str, @@ -253,6 +277,7 @@ def run( ] | None = None, external_finalizer: str | None = None, + check_current: Callable[[], None] | None = None, ) -> IsolatedWriteResult: if external_finalizer is not None: if not isinstance(external_finalizer, str) or not external_finalizer.strip(): @@ -267,6 +292,7 @@ def run( prepare_effects=prepare_effects, prepare_managed_artifacts=prepare_managed_artifacts, external_finalizer=external_finalizer, + check_current=check_current, ) self._ensure_repo_root() store = SemanticOperationStore(self.memory_root) @@ -280,6 +306,7 @@ def run( prepare_effects=prepare_effects, prepare_managed_artifacts=prepare_managed_artifacts, external_finalizer=external_finalizer, + check_current=check_current, ) def _run_claimed( @@ -295,6 +322,7 @@ def _run_claimed( ] | None = None, external_finalizer: str | None = None, + check_current: Callable[[], None] | None = None, ) -> IsolatedWriteResult: self._ensure_repo_root() operation_store: SemanticOperationStore | None = None @@ -317,7 +345,7 @@ def _run_claimed( f"prepared operation requires external finalizer: {recorded_finalizer}" ) return self._result_from_operation(operation) - return self._resume_prepared_operation(operation_store, operation) + return self._resume_prepared_operation(operation_store, operation, check_current=check_current) self._delete_operation_ref(operation_id) dirty = self._dirty_memory_files() @@ -365,7 +393,7 @@ def _run_claimed( semantic_changes = self._semantic_changes_between(start_head, current_head) if semantic_changes: detail = ", ".join(semantic_changes) - raise RuntimeError( + raise MemoryVersionConflictError( "main semantic state changed during isolated memory write: " + detail ) if commits: @@ -395,6 +423,8 @@ def _run_claimed( commits, managed_paths=frozenset(managed_paths), ) + if check_current is not None: + check_current() if operation_store is not None and operation_id is not None: if commits: self._pin_operation_ref(operation_id, commits[0]) @@ -467,12 +497,16 @@ def _resume_prepared_operation( self, store: SemanticOperationStore, operation: SemanticOperationRecord, + *, + check_current: Callable[[], None] | None = None, ) -> IsolatedWriteResult: outcome = operation.outcome if outcome is None: raise RuntimeError(f"prepared operation has no outcome: {operation.operation_id}") candidate = outcome.metadata.get("candidate_commit") with MemoryWriteLock(self.memory_root): + if check_current is not None: + check_current() current_head = self._git_stdout(self.memory_root, "rev-parse", "HEAD") if candidate is not None: if not isinstance(candidate, str) or not candidate: @@ -505,7 +539,7 @@ def _resume_prepared_operation( ) if semantic_changes: detail = ", ".join(semantic_changes) - raise RuntimeError( + raise MemoryVersionConflictError( "main semantic state changed before prepared memory write could land: " + detail ) @@ -522,7 +556,7 @@ def _resume_prepared_operation( ) if semantic_changes: detail = ", ".join(semantic_changes) - raise RuntimeError( + raise MemoryVersionConflictError( "main semantic state changed before prepared no-change outcome completed: " + detail ) @@ -778,7 +812,7 @@ def _rebase_operation_commit(self, worktree: Path, commit: str, new_base: str) - def _semantic_changes_between(self, start_commit: str, end_commit: str) -> list[str]: if not self._is_ancestor(start_commit, end_commit): - raise RuntimeError("main history diverged during isolated memory write") + raise MemoryVersionConflictError("main history diverged during isolated memory write") output = self._git_stdout( self.memory_root, "diff", diff --git a/rightmemory/prompts/assistant-retrieve.md b/rightmemory/prompts/assistant-retrieve.md new file mode 100644 index 0000000..7dee444 --- /dev/null +++ b/rightmemory/prompts/assistant-retrieve.md @@ -0,0 +1,19 @@ +# Retrieve for an Assistant + +Gather stored context that materially helps the caller understand or approach the current work. Return a concise handoff with sources; leave the user's task to the caller. + +## Inspect the Sources + +Use the supplied context's memory root, source paths, imported-view paths, and pending file with the assistant's native read and search tools. Follow the package schema when resolving graph references. Read enough to understand a relevant item's scope, evidence, and relationships; expand linked detail when the root description is insufficient. + +Consider durable Memory, the user's Pursuit map, and relevant Agent Corrections. Include guidance that could change the caller's next action. Agent Corrections belong in ordinary retrieval when the same failure pattern could matter. Root `corrections.md` is edit feedback for Update, not ordinary retrieval content. + +Treat Open Context Questions as questions. Treat pending candidates as unsettled evidence and preserve their status and provenance. An `MQ#` heading provides local relationship context; it does not authorize calling an external provider. + +## Hand Off What Matters + +Summarize useful context accurately and give the source file and heading or line range for consequential claims. Keep current stored state, pending evidence, inference, and unresolved conflicts distinguishable. If an `S#` resource is needed as an instruction, supply the complete instruction or direct the caller to its full file. + +Explain a material gap or inaccessible source rather than guessing its contents. Stop when further reading adds little to the caller's work. + +Keep retrieval read-only. diff --git a/rightmemory/recent_submitted.py b/rightmemory/recent_submitted.py index 2c6c970..d2675ff 100644 --- a/rightmemory/recent_submitted.py +++ b/rightmemory/recent_submitted.py @@ -1,5 +1,6 @@ from __future__ import annotations +from collections.abc import Iterable from contextlib import contextmanager from dataclasses import dataclass import json @@ -8,6 +9,7 @@ from .async_update import AsyncUpdateJob, AsyncUpdateState, AsyncUpdateStore from .platform import lock_file, unlock_file +from .semantic_operation import FINAL_PHASES, SemanticOperationStore from .session import _ensure_runtime_gitignore, _fsync_directory, _safe_session_id from .update_queue import UpdateQueueStore @@ -145,10 +147,69 @@ def collect_recent_submitted_memory(memory_root: Path) -> list[RecentSubmittedMe candidate_uid=candidate.uid, ) ) - # A publishing device may temporarily see the same immutable candidate in both lanes. - entries = list({entry.key: entry for entry in entries}.values()) + return _unique_sorted_entries(entries) + + +def collect_recent_submitted_memory_readonly( + memory_root: Path, + *, + excluded_candidate_uids: Iterable[str] = (), +) -> list[RecentSubmittedMemoryEntry]: + """Read pending evidence without recovering sessions or inspecting workers. + + Terminal operation receipts take precedence over an unacknowledged current + batch, including copies still present in the local outbox or tracked queue. + State locks may be created, but candidate and session records are unchanged. + """ + store = AsyncUpdateStore(memory_root, "update") + operations = SemanticOperationStore(memory_root) + excluded = set(excluded_candidate_uids) + entries: list[RecentSubmittedMemoryEntry] = [] + for state_path in store._session_state_paths(): + with store._locked(state_path.stem): + state = store._read_raw(state_path.stem) + if state.role != "update": + raise ValueError(f"async update state role mismatch: expected update, got {state.role}") + completed = False + if state.current_batch and state.current_operation_id is not None: + operation = operations.read(state.current_operation_id) + completed = ( + operation is not None + and operation.phase in FINAL_PHASES + and operation.outcome is not None + ) + if completed: + excluded.update(job.candidate_uid for job in state.current_batch) + else: + entries.extend(_entries_from_jobs(state, state.current_batch)) + entries.extend(_entries_from_jobs(state, state.pending)) + + queue_store = UpdateQueueStore(memory_root) + for candidate in ( + *queue_store.outbox_candidates(cleanup_temporary=False), + *queue_store.snapshot().candidates, + ): + entries.append( + RecentSubmittedMemoryEntry( + update_session_id=candidate.session_id, + candidate_id=candidate.display_id, + submitted_at=candidate.submitted_at, + message=candidate.message, + candidate_uid=candidate.uid, + ) + ) + return _unique_sorted_entries( + entry for entry in entries if entry.candidate_uid not in excluded + ) + + +def _unique_sorted_entries( + entries: Iterable[RecentSubmittedMemoryEntry], +) -> list[RecentSubmittedMemoryEntry]: + # Publication temporarily exposes the same immutable candidate in both lanes. + unique = {entry.key: entry for entry in entries} return sorted( - entries, + unique.values(), key=lambda entry: ( entry.submitted_at, entry.update_session_id, diff --git a/rightmemory/update_queue.py b/rightmemory/update_queue.py index ba9904b..60d2a3e 100644 --- a/rightmemory/update_queue.py +++ b/rightmemory/update_queue.py @@ -220,7 +220,7 @@ def read_outbox(self, uid: str) -> UpdateCandidate | None: ) return candidate - def outbox_candidates(self) -> tuple[UpdateCandidate, ...]: + def outbox_candidates(self, *, cleanup_temporary: bool = True) -> tuple[UpdateCandidate, ...]: if not self.outbox_root.exists() and not self.outbox_root.is_symlink(): return () if self.outbox_root.is_symlink() or not self.outbox_root.is_dir(): @@ -235,7 +235,7 @@ def outbox_candidates(self) -> tuple[UpdateCandidate, ...]: raise UpdateQueueFormatError( f"local outbox temporary artifact must be a regular file: {path}" ) - if not process_exists(int(temporary.group("pid"))): + if cleanup_temporary and not process_exists(int(temporary.group("pid"))): _unlink(path) continue if _OUTBOX_RECORD_RE.fullmatch(path.name) is None: diff --git a/rightmemory/update_queue_git.py b/rightmemory/update_queue_git.py index aee4aed..7fe1073 100644 --- a/rightmemory/update_queue_git.py +++ b/rightmemory/update_queue_git.py @@ -123,6 +123,22 @@ def __init__(self, config: SyncConfig, *, device_id: str | None = None): self.store = UpdateQueueStore(self.memory_root) self.device_id = device_id or _load_device_id(self.memory_root) + def ready_candidates( + self, *, trigger_candidates: int, target_batch_candidates: int, + max_wait_seconds: int, now: datetime | None = None, + ) -> tuple[tuple[UpdateCandidate, ...], datetime | None]: + """Inspect the last synchronized queue without acquiring its lease.""" + now = _utc_now() if now is None else now.astimezone(UTC) + snapshot = self.store.snapshot() + if snapshot.lease is not None and not _lease_expired(snapshot.lease, now): + return (), _parse_time(snapshot.lease.expires_at) + candidates, deadline = _select_candidates( + snapshot, trigger_candidates=trigger_candidates, + target_batch_candidates=target_batch_candidates, + max_wait_seconds=max_wait_seconds, now=now, + ) + return tuple(candidates), deadline + def publish_outbox( self, candidate_uids: set[str] | frozenset[str] | None = None, @@ -420,7 +436,13 @@ def _finalize_locked( execution_locked=True, ): return recovered - raise UpdateQueueLeaseLost("synchronized update finalization lost its Git lease") + if upstream is None: + raise UpdateQueueUnavailable("cannot confirm synchronized finalization while Git is offline") + if recovered is not None: + raise UpdateQueueUnavailable("published update could not advance the active checkout") + with self._worktree(upstream.commit) as worktree: + self._require_claim(UpdateQueueStore(worktree).snapshot(), claim) + raise UpdateQueueUnavailable("synchronized update is prepared but Git publication has not succeeded") def release(self, claim: ClaimedUpdateBatch) -> None: self._settle_failed_claim(claim, recovery=None) diff --git a/rightmemory/validation.py b/rightmemory/validation.py new file mode 100644 index 0000000..2a2701f --- /dev/null +++ b/rightmemory/validation.py @@ -0,0 +1,98 @@ +from __future__ import annotations + +import os +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from .guidance import GUIDANCE_INBOX_PATH, validate_guidance_inbox +from .tools import MemoryTools + + +@dataclass(frozen=True) +class ValidationResult: + ok: bool + errors: tuple[str, ...] + report: str + memory_report: str | None + document_errors: tuple[str, ...] = () + guidance_errors: tuple[str, ...] = () + + def to_dict(self) -> dict[str, Any]: + return { + "ok": self.ok, + "errors": list(self.errors), + "report": self.report, + "memory_report": self.memory_report, + "document_errors": list(self.document_errors), + "guidance_errors": list(self.guidance_errors), + } + + +def guidance_validation_errors(memory_root: Path) -> list[str]: + """Validate the optional pending guidance document without changing it.""" + path = Path(memory_root) / GUIDANCE_INBOX_PATH + if not path.exists() and not path.is_symlink(): + return [] + if path.is_symlink() or not path.is_file(): + return [f"{GUIDANCE_INBOX_PATH} must be a regular file"] + return validate_guidance_inbox(path.read_text(encoding="utf-8")) + + +def validate_memory_root(memory_root: Path) -> ValidationResult: + """Run the complete standalone validation boundary without a model or writes.""" + # Sync also consumes the root document contract. Import lazily so this + # validator can be used by publication boundaries without an import cycle. + from .sync import LEGACY_ROOT_REFERENCE_PATHS, REQUIRED_ROOT_DOCUMENTS + + root = Path(memory_root).expanduser().resolve() + document_errors = [ + f"required root document is not a regular file: {name}" + for name in REQUIRED_ROOT_DOCUMENTS + if (root / name).is_symlink() or not (root / name).is_file() + ] + document_errors.extend( + f"legacy package reference must be removed from the root: {name}" + for name in sorted(LEGACY_ROOT_REFERENCE_PATHS) + if os.path.lexists(root / name) + ) + corrections_path = root / "corrections.md" + if corrections_path.is_symlink() or ( + corrections_path.exists() and not corrections_path.is_file() + ): + document_errors.append("optional root document is not a regular file: corrections.md") + + memory_report = None + errors = list(document_errors) + if not document_errors: + try: + memory_report = MemoryTools(root, role="sync-reconciler").validate_memory() + except (OSError, UnicodeError) as exc: + errors.append(f"cannot read RightMemory documents: {exc}") + else: + if not memory_report.startswith("validation passed:"): + # MemoryTools owns validation and its original report remains + # available to callers; expose the same diagnostics as data. + diagnostics = memory_report.removeprefix("validation failed:\n- ") + errors.extend(diagnostics.split("\n- ")) + + try: + guidance_errors = guidance_validation_errors(root) + except (OSError, UnicodeError) as exc: + guidance_errors = [f"cannot read {GUIDANCE_INBOX_PATH}: {exc}"] + errors.extend(guidance_errors) + ok = not errors + report = ( + memory_report + if ok + else "validation failed:\n" + "\n".join(f"- {error}" for error in errors) + ) + assert report is not None + return ValidationResult( + ok=ok, + errors=tuple(errors), + report=report, + memory_report=memory_report, + document_errors=tuple(document_errors), + guidance_errors=tuple(guidance_errors), + ) diff --git a/skills/rightmemory-assistant/SKILL.md b/skills/rightmemory-assistant/SKILL.md new file mode 100644 index 0000000..e05e454 --- /dev/null +++ b/skills/rightmemory-assistant/SKILL.md @@ -0,0 +1,56 @@ +--- +name: rightmemory-assistant +description: "Use when the user chooses RightMemory assistant execution: retrieve context and reconcile submitted evidence through the current assistant's own agents and file tools." +--- + +# Use RightMemory with Native Agents + +Use the configured RightMemory root and one stable session id throughout the conversation. Use its absolute path as `` in every assistant command. The main agent owns submissions and Update delegation. Retrieve and Update run as native subagents with access to the supplied directories; RightMemory tools manage the queue, isolation, validation, and publication. + +Use the `rightmemory assistant` commands below, or their matching assistant MCP tools. Keep this skill active with either transport. + +## Retrieve Useful Context + +Retrieve when stored context could materially change how you understand or approach the work. Delegate that need to a native Retrieve subagent, with the bound root, session id, and this preparation command (or matching MCP tool): + +```bash +rightmemory assistant --root context --session +``` + +The Retrieve subagent calls `context`, reads the returned `instructions_file`, then uses native file tools to inspect the real sources and `pending_file`. It returns useful context with its sources. Supply prior context when it would avoid redundant reading. + +Reconcile the handoff with the current conversation and evidence. Apply relevant guidance in the work; current user instructions take precedence over stale or conflicting memory. Submit concrete evidence of wrong, stale, or misleading Memory or Agent Corrections when you find it. + +## Submit Evidence + +Submit evidence when losing it would impair future decisions, cause substantial rediscovery, or repeat a reusable agent failure. Preserve the circumstances, supporting evidence, and unresolved uncertainty that affect its meaning. For user redirections, describe the prior approach, the user's signal, and the resulting direction. Leave final stored wording and placement to Update. + +Combine related evidence ready at the same boundary. Routine progress and details already adequately preserved in project artifacts rarely need submission. Pursuit is read-only context; explicit map edits use the separate maintenance workflow. + +Write the evidence to a UTF-8 file outside the memory root, then submit it: + +```bash +rightmemory assistant --root submit --session --request-id --file +``` + +Reuse the same request id and evidence when retrying the same submission. Use a new id for new evidence. A saved candidate is pending evidence, not confirmation that it became memory. + +## Reconcile Ready Batches + +Act on batch-ready notices returned by normal RightMemory tool calls. A subagent that encounters one passes it to the main agent with its result. The main agent obtains the next batch with: + +```bash +rightmemory assistant --root update begin +``` + +Delegate a newly claimed task (`claimed: true`) with its instructions, candidates, and isolated worktree to one native Update subagent. It follows the canonical Update role, edits allowed files in that worktree, validates and commits them, and calls `update complete` for the task. Retain the root binding supplied with the task when calling control commands from the worktree. An existing task (`claimed: false`) must not be delegated twice; a sync problem must be resolved before Update begins. + +Continue the user's work while Update runs. Treat an already active batch as existing work. Use `rightmemory assistant --root update status --task ` to resolve an uncertain result before dispatching another updater. + +## Report the Actual Outcome + +Only the program's completion receipt confirms that candidate work reached the active root. A prepared candidate is not a completed update. On interruption or failed publication, inspect task status and retry completion with the same task id and worktree; a prepared result retries publication without repeating semantic Update. + +When a stale task must be reconciled anew, `rightmemory assistant --root update begin --retry ` explicitly replaces that task for the same batch and fences its old edits. Use this only after deciding to abandon those edits, rather than for an uncertain completion response. + +Report an inaccessible directory or unsupported native delegation as an assistant limitation. Do not substitute another agent product or claim adaptation from MCP tool discovery alone. diff --git a/tests/test_assistant_cli.py b/tests/test_assistant_cli.py new file mode 100644 index 0000000..9933bbc --- /dev/null +++ b/tests/test_assistant_cli.py @@ -0,0 +1,100 @@ +import contextlib +import io +import json +import os +import subprocess +import sys +from pathlib import Path + +from rightmemory.entrypoint import main +from tests.isolated_write_test_base import IsolatedWriteTestBase + + +class AssistantCliTests(IsolatedWriteTestBase): + def setUp(self): + super().setUp() + (self.root / "rightmemory.toml").write_text( + "[update.async]\ntrigger_candidates = 1\ntarget_batch_candidates = 1\nmax_wait_seconds = 1\n" + ) + + def _call(self, *args): + output = io.StringIO() + with contextlib.redirect_stdout(output): + code = main(["assistant", "--root", str(self.root), *args]) + return code, json.loads(output.getvalue()) + + def test_cli_native_flow_binds_completion_to_active_root_from_worktree(self): + evidence = self.root.parent / f"{self.root.name}-evidence.txt" + evidence.write_text("Fictional project launch code is Atlas.") + self.addCleanup(evidence.unlink, missing_ok=True) + code, submitted = self._call("submit", "--session", "cli", "--request-id", "one", "--file", str(evidence)) + self.assertEqual(code, 0) + self.assertTrue(submitted["update"]["ready"]) + code, task = self._call("update", "begin") + self.assertEqual(code, 0) + worktree = Path(task["worktree"]) + self._append_memory(worktree, "- `atlas` Fictional project launch code is Atlas. → []\n") + self._git("add", "MEMORY.md", cwd=worktree) + self._git("commit", "-m", "memory: CLI native edit", cwd=worktree) + environment = {**os.environ, "PYTHONPATH": str(Path(__file__).resolve().parents[1])} + result = subprocess.run( + [sys.executable, "-m", "rightmemory.entrypoint", *task["complete_argv"][1:]], + cwd=worktree, env=environment, text=True, capture_output=True, timeout=30, + ) + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + receipt = json.loads(result.stdout) + self.assertEqual(receipt["status"], "completed") + self.assertEqual(receipt["landed_commit"], self._git("rev-parse", "HEAD")) + self.assertIn("Atlas", (self.root / "MEMORY.md").read_text()) + + def test_argument_and_core_errors_are_json_with_failure_exit(self): + code, result = self._call("submit", "--session", "cli") + self.assertEqual(code, 1) + self.assertFalse(result["ok"]) + self.assertEqual(result["error"]["type"], "ValueError") + code, result = self._call("update", "complete", "--task", "../../outside") + self.assertEqual(code, 1) + self.assertFalse(result["ok"]) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + def test_validate_is_independent_and_reports_invalid_root(self): + code, result = self._call("validate") + self.assertEqual(code, 0) + self.assertTrue(result["ok"]) + (self.root / "PURSUITS.md").unlink() + code, result = self._call("validate") + self.assertEqual(code, 1) + self.assertFalse(result["ok"]) + self.assertTrue(result["document_errors"]) + + def test_new_entry_points_do_not_import_any_model_runtime(self): + script = r''' +import contextlib +import importlib.abc +import io +import json +import sys + +class RejectModelRuntime(importlib.abc.MetaPathFinder): + def find_spec(self, fullname, path=None, target=None): + blocked = ("pydantic_ai", "rightmemory.cli", "rightmemory.runtime", "rightmemory.codex_sdk", "rightmemory.agent_cli") + if any(fullname == name or fullname.startswith(name + ".") for name in blocked): + raise ImportError("unexpected model/runtime dependency: " + fullname) + +sys.meta_path.insert(0, RejectModelRuntime()) +from rightmemory.entrypoint import main +root = sys.argv[1] +with contextlib.redirect_stdout(io.StringIO()): + assert main(["validate", "--root", root]) == 0 +for command in (["context", "--session", "reader"], ["update", "status"], ["sync", "refresh"], ["validate"]): + captured = io.StringIO() + with contextlib.redirect_stdout(captured): + assert main(["assistant", "--root", root, *command]) == 0 + assert json.loads(captured.getvalue())["ok"] +print("ok") +''' + result = subprocess.run( + [sys.executable, "-c", script, str(self.root)], text=True, capture_output=True, timeout=30, + ) + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + self.assertEqual(result.stdout.strip(), "ok") diff --git a/tests/test_assistant_context.py b/tests/test_assistant_context.py new file mode 100644 index 0000000..f85e8dc --- /dev/null +++ b/tests/test_assistant_context.py @@ -0,0 +1,257 @@ +from __future__ import annotations + +import json +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from rightmemory.assistant_context import prepare_retrieve_context +from rightmemory.async_update import AsyncUpdateJob, AsyncUpdateState, AsyncUpdateStore +from rightmemory.recent_submitted import collect_recent_submitted_memory_readonly +from rightmemory.semantic_operation import SemanticOperationStore +from rightmemory.shared_view_files import FileViewPullResult +from rightmemory.sync import SyncResult +from rightmemory.update_queue import UpdateCandidate, UpdateQueueStore + + +class AssistantRetrieveContextTests(unittest.TestCase): + def setUp(self) -> None: + self.tempdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tempdir.cleanup) + self.root = Path(self.tempdir.name) + (self.root / "MEMORY.md").write_text("# Memory\n", encoding="utf-8") + (self.root / "PURSUITS.md").write_text("# Pursuits\n", encoding="utf-8") + + def prepare(self, **kwargs): + # Injection keeps these boundary tests independent of role text. + return prepare_retrieve_context(self.root, instructions="test instructions", **kwargs) + + def test_prepares_native_sources_without_copying_the_root(self) -> None: + (self.root / "MEMORY.md").write_text( + "# Memory\n\n## Details {F#details}\n\n" + "## Notes {M#notes}\n\n## Skill {S#review}\n", + encoding="utf-8", + ) + for name, content in { + "MEMORY_details.md": "- `fact` Evidence. → []\n", + "MEMORY_notes.md": "# Notes\n", + "MEMORY_SKILL_review.md": "# Review\n", + "MEMORY_agent-corrections-writing.md": "# Writing\n", + }.items(): + (self.root / name).write_text(content, encoding="utf-8") + + result = self.prepare() + + self.assertEqual(Path(result["memory_root"]), self.root.resolve()) + self.assertTrue(result["live_files"]) + self.assertIsNone(result["memory_commit"]) + self.assertEqual( + {Path(path).name for path in result["source_files"]}, + { + "MEMORY.md", "PURSUITS.md", "MEMORY_details.md", "MEMORY_notes.md", + "MEMORY_SKILL_review.md", "MEMORY_agent-corrections-writing.md", + }, + ) + self.assertTrue(all(Path(path).parent == self.root for path in result["source_files"])) + self.assertTrue(Path(result["pending_file"]).is_file()) + self.assertTrue(Path(result["instructions_file"]).is_file()) + self.assertEqual(len(list(Path(result["pending_file"]).parent.iterdir())), 2) + self.assertEqual(result["pending_count"], 0) + self.assertEqual(result["sync"]["status"], "disabled") + self.assertIsInstance(json.loads(json.dumps(result)), dict) + + def test_refreshes_sync_then_file_views_without_worker_calls(self) -> None: + calls = [] + + def refresh(): + calls.append("sync") + return SyncResult("fresh", "test") + + def pull(root): + calls.append("file_views") + self.assertEqual(root, self.root) + return [FileViewPullResult("remote", "unavailable", "test")] + + with ( + patch("rightmemory.assistant_context.SyncManager.refresh_for_retrieve", side_effect=refresh), + patch("rightmemory.assistant_context.pull_all_file_views", side_effect=pull), + patch.object(AsyncUpdateStore, "wake_worker", side_effect=AssertionError("worker called")), + patch.object(AsyncUpdateStore, "_worker_snapshot", side_effect=AssertionError("worker inspected")), + ): + result = self.prepare() + + self.assertEqual(calls, ["sync", "file_views"]) + self.assertEqual(result["sync"]["status"], "fresh") + self.assertEqual(result["file_views"][0]["status"], "unavailable") + + def test_sync_failure_preserves_available_local_sources(self) -> None: + with patch( + "rightmemory.assistant_context.SyncManager.refresh_for_retrieve", + side_effect=OSError("offline"), + ): + result = self.prepare() + + self.assertEqual(result["sync"]["status"], "error") + self.assertEqual(len(result["source_files"]), 2) + self.assertTrue(Path(result["pending_file"]).is_file()) + + def test_context_records_the_observed_git_version(self) -> None: + commit = "a" * 40 + with patch("rightmemory.assistant_context.current_memory_head", return_value=commit): + result = self.prepare() + self.assertEqual(result["memory_commit"], commit) + self.assertTrue(result["live_files"]) + + def test_pending_snapshot_filters_receipts_and_has_an_independent_path(self) -> None: + queue = UpdateQueueStore(self.root) + queue.write_outbox(_candidate(1)) + queue.write_candidate(_candidate(2)) + + first = self.prepare(excluded_candidate_uids=[_candidate(1).uid]) + second = self.prepare() + + self.assertEqual(first["pending_count"], 1) + self.assertEqual(second["pending_count"], 2) + self.assertNotEqual(first["pending_file"], second["pending_file"]) + self.assertTrue(Path(first["pending_file"]).is_file()) + + def test_mf_imports_point_to_the_existing_validated_dist(self) -> None: + (self.root / "MEMORY.md").write_text("# Remote {MF#remote}\n", encoding="utf-8") + package = self.root / ".runtime" / "shared_views" / "imports" / "remote" + (package / "dist").mkdir(parents=True) + files = { + "view.md": "# Remote\n", + "recipe.toml": 'version = 1\nview_id = "remote"\nkind = "file"\n', + "rightmemory-shared-view.toml": 'version = 2\nview_id = "remote"\nkind = "file"\n', + "dist/manifest.toml": ( + 'version = 2\nview_id = "remote"\ndocument_kind = "rightmemory-memory"\n' + ), + "dist/MEMORY.md": "# Published {#published} → []\n", + } + for relative, content in files.items(): + (package / relative).write_text(content, encoding="utf-8") + + result = self.prepare() + + self.assertEqual(result["mf_imports"], {"remote": str(package / "dist")}) + (package / "dist" / "manifest.toml").unlink() + self.assertEqual(self.prepare()["mf_imports"], {}) + + def test_missing_root_is_not_created(self) -> None: + missing = self.root / "missing" + with self.assertRaises(FileNotFoundError): + prepare_retrieve_context(missing, instructions="test instructions") + self.assertFalse(missing.exists()) + + +class ReadonlyRecentSubmissionsTests(unittest.TestCase): + def setUp(self) -> None: + self.tempdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tempdir.cleanup) + self.root = Path(self.tempdir.name) + self.store = AsyncUpdateStore(self.root, "update") + + def write_state(self, *, operation_id=None) -> Path: + self.store._write( + "session", + AsyncUpdateState( + status="running", + phase="running", + session_id="session", + role="update", + pid=999999, + current_operation_id=operation_id, + current_batch=[_job(1)], + pending=[_job(2)], + next_id=3, + ), + ) + return self.store._state_path("session") + + def test_reads_current_pending_and_both_queue_lanes_without_worker_recovery(self) -> None: + state_path = self.write_state() + before = state_path.read_bytes() + queue = UpdateQueueStore(self.root) + queue.write_candidate(_candidate(1)) + queue.write_outbox(_candidate(1)) + queue.write_candidate(_candidate(3)) + queue.write_outbox(_candidate(4)) + + with ( + patch.object(AsyncUpdateStore, "_read_checked_locked", side_effect=AssertionError("mutating read")), + patch.object(AsyncUpdateStore, "_worker_snapshot", side_effect=AssertionError("worker inspected")), + ): + entries = collect_recent_submitted_memory_readonly(self.root) + + self.assertEqual([entry.candidate_uid for entry in entries], [_candidate(i).uid for i in range(1, 5)]) + self.assertEqual(state_path.read_bytes(), before) + + def test_terminal_operation_filters_unacknowledged_batch_from_every_lane(self) -> None: + operations = SemanticOperationStore(self.root) + operation_id = "completed-update" + operations.begin(operation_id, {"role": "update"}) + operations.prepare_outcome(operation_id, output="", start_commit="a" * 40, changed_paths=[]) + operations.complete_no_change(operation_id) + state_path = self.write_state(operation_id=operation_id) + before = state_path.read_bytes() + queue = UpdateQueueStore(self.root) + queue.write_outbox(_candidate(1)) + queue.write_candidate(_candidate(1)) + + entries = collect_recent_submitted_memory_readonly(self.root) + + self.assertEqual([entry.candidate_uid for entry in entries], [_candidate(2).uid]) + self.assertEqual(state_path.read_bytes(), before) + self.assertIsNotNone(queue.read_outbox(_candidate(1).uid)) + self.assertIsNotNone(queue.read_candidate(_candidate(1).uid)) + + def test_explicit_receipt_exclusion_covers_all_lanes(self) -> None: + self.write_state() + queue = UpdateQueueStore(self.root) + queue.write_outbox(_candidate(1)) + queue.write_candidate(_candidate(2)) + queue.write_outbox(_candidate(3)) + + entries = collect_recent_submitted_memory_readonly( + self.root, excluded_candidate_uids=[_candidate(1).uid, _candidate(3).uid] + ) + + self.assertEqual([entry.candidate_uid for entry in entries], [_candidate(2).uid]) + + def test_readonly_outbox_does_not_reap_temporary_artifacts(self) -> None: + queue = UpdateQueueStore(self.root) + queue.outbox_root.mkdir(parents=True) + temporary = queue.outbox_root / f".999999.{('a' * 32)}.tmp" + temporary.write_bytes(b"{}") + + with patch("rightmemory.update_queue.process_exists", side_effect=AssertionError("process inspected")): + entries = collect_recent_submitted_memory_readonly(self.root) + + self.assertEqual(entries, []) + self.assertTrue(temporary.is_file()) + + def test_malformed_state_is_not_silently_dropped(self) -> None: + self.store.root.mkdir(parents=True) + (self.store.root / "session.json").write_text("[]", encoding="utf-8") + with self.assertRaises(ValueError): + collect_recent_submitted_memory_readonly(self.root) + + +def _candidate(number: int) -> UpdateCandidate: + return UpdateCandidate( + uid=f"{number:032x}", + session_id="session", + display_id=number, + message="test evidence", + submitted_at=f"2026-05-19T00:00:{number:02d}+00:00", + ) + + +def _job(number: int) -> AsyncUpdateJob: + candidate = _candidate(number) + return AsyncUpdateJob(candidate.display_id, candidate.uid, candidate.message, candidate.submitted_at) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_assistant_mcp.py b/tests/test_assistant_mcp.py new file mode 100644 index 0000000..9bf54b3 --- /dev/null +++ b/tests/test_assistant_mcp.py @@ -0,0 +1,234 @@ +from __future__ import annotations + +import asyncio +import sys +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +from mcp import Client, ClientSession, StdioServerParameters, stdio_client +from mcp.types import CallToolResult + +from rightmemory.assistant_mcp import create_mcp_server, mcp_main +from tests.isolated_write_test_base import IsolatedWriteTestBase + + +class FakeBackend: + def __init__(self) -> None: + self.calls: list[tuple] = [] + self.result = {"ok": True, "ready": {"batch_id": "batch-1"}} + self.error: Exception | None = None + + def _record(self, *call): + self.calls.append(call) + if self.error is not None: + raise self.error + return self.result + + def context(self, session_id): + return self._record("context", session_id) + + def submit(self, session_id, request_id, evidence): + return self._record("submit", session_id, request_id, evidence) + + def begin_update(self, retry_task=None): + return self._record("begin_update", retry_task) + + def complete_update(self, task_id, output=""): + return self._record("complete_update", task_id, output) + + def update_status(self, task_id=None): + return self._record("update_status", task_id) + + def sync(self, action="refresh"): + return self._record("sync", action) + + +async def _call(server, name, arguments): + async with Client(server, raise_exceptions=True) as client: + return await client.call_tool(name, arguments) + + +def call(server, name, arguments) -> CallToolResult: + result = asyncio.run(_call(server, name, arguments)) + if not isinstance(result, CallToolResult): + raise AssertionError(type(result)) + return result + + +async def _list(server): + async with Client(server, raise_exceptions=True) as client: + return (await client.list_tools()).tools + + +class AssistantMcpTests(unittest.TestCase): + def setUp(self): + self.backend = FakeBackend() + self.root = Path("/unused-assistant-root").resolve() + self.server = create_mcp_server(self.root, backend=self.backend) + + def test_exposes_the_assistant_operations(self): + self.assertEqual( + {tool.name for tool in asyncio.run(_list(self.server))}, + { + "rightmemory_assistant_context", + "rightmemory_assistant_submit", + "rightmemory_assistant_begin_update", + "rightmemory_assistant_complete_update", + "rightmemory_assistant_update_status", + "rightmemory_assistant_sync", + "rightmemory_assistant_validate", + }, + ) + + def test_routes_operations_without_rewriting_ids_evidence_or_receipts(self): + cases = [ + ("context", {"session_id": "s"}, ("context", "s")), + ( + "submit", + {"session_id": "s", "request_id": "r", "evidence": " evidence\n"}, + ("submit", "s", "r", " evidence\n"), + ), + ("begin_update", {}, ("begin_update", None)), + ("begin_update", {"retry_task": "old"}, ("begin_update", "old")), + ( + "complete_update", + {"task_id": "t", "output": "summary"}, + ("complete_update", "t", "summary"), + ), + ("complete_update", {"task_id": "t"}, ("complete_update", "t", "")), + ("update_status", {}, ("update_status", None)), + ("update_status", {"task_id": "t"}, ("update_status", "t")), + ("sync", {}, ("sync", "refresh")), + ("sync", {"action": "push"}, ("sync", "push")), + ] + for name, arguments, expected in cases: + with self.subTest(name=name, arguments=arguments): + result = call(self.server, f"rightmemory_assistant_{name}", arguments) + self.assertFalse(result.is_error) + self.assertEqual(self.backend.calls[-1], expected) + self.assertEqual(result.structured_content, self.backend.result) + + def test_reports_core_failures_as_json_errors(self): + for error in (ValueError("bad input"), RuntimeError("conflict"), FileNotFoundError("task")): + with self.subTest(error=type(error).__name__): + self.backend.error = error + result = call(self.server, "rightmemory_assistant_context", {"session_id": "s"}) + self.assertTrue(result.is_error) + self.assertEqual( + result.structured_content, + {"ok": False, "error": {"type": type(error).__name__, "message": str(error)}}, + ) + + def test_preserves_an_unsuccessful_core_result(self): + self.backend.result = {"ok": False, "status": "conflict", "task_id": "t"} + result = call(self.server, "rightmemory_assistant_complete_update", {"task_id": "t"}) + self.assertTrue(result.is_error) + self.assertEqual(result.structured_content, self.backend.result) + + def test_missing_required_input_does_not_call_the_core(self): + result = call(self.server, "rightmemory_assistant_submit", {"session_id": "s"}) + self.assertTrue(result.is_error) + self.assertEqual(self.backend.calls, []) + + def test_invalid_sync_action_does_not_call_the_core(self): + result = call(self.server, "rightmemory_assistant_sync", {"action": "erase"}) + self.assertTrue(result.is_error) + self.assertEqual(self.backend.calls, []) + + def test_validation_uses_only_the_bound_root(self): + result_data = {"ok": False, "errors": ["invalid"]} + with patch("rightmemory.assistant_mcp.validate_memory_root") as validate: + validate.return_value = SimpleNamespace(to_dict=lambda: result_data) + result = call(self.server, "rightmemory_assistant_validate", {}) + validate.assert_called_once_with(self.root) + self.assertTrue(result.is_error) + self.assertEqual(result.structured_content, result_data) + self.assertEqual(self.backend.calls, []) + + def test_stdio_entrypoint_selects_the_bound_root(self): + with patch("rightmemory.assistant_mcp.create_mcp_server") as factory: + result = mcp_main(self.root, ["--transport", "stdio"]) + self.assertEqual(result, 0) + factory.assert_called_once_with(self.root) + factory.return_value.run.assert_called_once_with(transport="stdio") + + +class AssistantMcpStdioTests(IsolatedWriteTestBase): + def test_entrypoint_stdio_handshake_and_model_free_core_calls(self): + (self.root / "rightmemory.toml").write_text( + "[update.async]\ntrigger_candidates = 1\n" + "target_batch_candidates = 1\nmax_wait_seconds = 1\n", + encoding="utf-8", + ) + parameters = StdioServerParameters( + command=sys.executable, + args=[ + "-m", "rightmemory.entrypoint", "assistant", "--root", str(self.root), + "mcp", "--transport", "stdio", + ], + cwd=Path(__file__).resolve().parents[1], + ) + evidence = "The fictional Atlas project keeps its blue test fixture in the west room." + + async def exercise(): + async with stdio_client(parameters) as (reader, writer): + async with ClientSession(reader, writer, read_timeout_seconds=10) as client: + initialized = await client.initialize() + self.assertEqual(initialized.server_info.name, "RightMemory Assistant") + listed = await client.list_tools() + self.assertEqual(len(listed.tools), 7) + + result = await client.call_tool( + "rightmemory_assistant_context", {"session_id": "stdio-session"} + ) + self.assertFalse(result.is_error) + context = result.structured_content + self.assertEqual(context["memory_root"], str(self.root.resolve())) + self.assertEqual(context["memory_commit"], self.initial_head) + self.assertEqual(context["pending_count"], 0) + self.assertTrue(Path(context["instructions_file"]).is_file()) + + arguments = { + "session_id": "stdio-session", "request_id": "evidence-1", + "evidence": evidence, + } + first = await client.call_tool("rightmemory_assistant_submit", arguments) + self.assertFalse(first.is_error) + self.assertTrue(first.structured_content["saved"]) + self.assertFalse(first.structured_content["duplicate"]) + self.assertTrue(first.structured_content["update"]["ready"]) + + repeated = await client.call_tool("rightmemory_assistant_submit", arguments) + self.assertFalse(repeated.is_error) + self.assertTrue(repeated.structured_content["duplicate"]) + self.assertEqual( + repeated.structured_content["candidate_uid"], + first.structured_content["candidate_uid"], + ) + rejected = await client.call_tool( + "rightmemory_assistant_submit", {**arguments, "evidence": "Different fact."} + ) + self.assertTrue(rejected.is_error) + self.assertEqual(rejected.structured_content["error"]["type"], "ValueError") + + pending = await client.call_tool( + "rightmemory_assistant_context", {"session_id": "stdio-session"} + ) + self.assertFalse(pending.is_error) + self.assertEqual(pending.structured_content["pending_count"], 1) + self.assertIn( + evidence, + Path(pending.structured_content["pending_file"]).read_text(encoding="utf-8"), + ) + validated = await client.call_tool("rightmemory_assistant_validate", {}) + self.assertFalse(validated.is_error) + self.assertTrue(validated.structured_content["ok"]) + + asyncio.run(exercise()) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_assistant_queue.py b/tests/test_assistant_queue.py new file mode 100644 index 0000000..0bcd451 --- /dev/null +++ b/tests/test_assistant_queue.py @@ -0,0 +1,203 @@ +from __future__ import annotations + +import tempfile +import unittest +from dataclasses import replace +from pathlib import Path +from unittest.mock import Mock, patch + +from rightmemory.async_update import AsyncUpdateStore +from rightmemory.update_queue import UpdateQueueStore + + +class AssistantQueueTests(unittest.TestCase): + def setUp(self) -> None: + self.tempdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tempdir.cleanup) + self.root = Path(self.tempdir.name) + self.assistant = AsyncUpdateStore(self.root, "update", executor="assistant") + self.worker = AsyncUpdateStore(self.root, "update") + + def submit(self, session: str, uid_number: int): + return self.assistant.submit(session, "test evidence", candidate_uid=_uid(uid_number)) + + def ready(self, store=None, *, trigger=1, target=1): + return (store or self.assistant).ready_batch( + trigger_candidates=trigger, target_batch_candidates=target, max_wait_seconds=0 + ) + + def reserve(self, *, trigger=1, target=1): + batch, deadline = self.ready(trigger=trigger, target=target) + self.assertIsNone(deadline) + self.assertIsNotNone(batch) + reservation = self.assistant.reserve_batch(batch) + self.assertIsNotNone(reservation) + return reservation + + def test_assistant_submit_persists_ownership_without_starting_or_inspecting_a_worker(self) -> None: + with ( + patch.object(AsyncUpdateStore, "_start_worker_if_needed", side_effect=AssertionError("worker started")), + patch.object(AsyncUpdateStore, "_active_worker_pid", side_effect=AssertionError("worker inspected")), + ): + submitted = self.submit("assistant-session", 1) + loaded = self.assistant.read("assistant-session") + + self.assertEqual(submitted.executor, "assistant") + self.assertEqual(loaded.executor, "assistant") + self.assertEqual([job.candidate_uid for job in loaded.pending], [_uid(1)]) + self.assertIsNone(loaded.pid) + self.assertFalse(self.assistant._worker_state_path().exists()) + + def test_default_worker_submit_and_execution_remain_available(self) -> None: + with patch.object(self.worker, "_start_worker_if_needed") as start: + submitted = self.worker.submit("worker-session", "test evidence", candidate_uid=_uid(1)) + start.assert_called_once_with("worker-session") + self.assertEqual(submitted.executor, "worker") + callback = Mock(return_value="completed") + + result = self.worker.run_pending_batches( + callback, trigger_candidates=1, target_batch_candidates=1, max_wait_seconds=0 + ) + + callback.assert_called_once() + self.assertEqual(result.processed, 1) + self.assertEqual(self.worker.read("worker-session").pending, []) + + def test_cross_session_batch_uses_the_existing_aggregate_trigger(self) -> None: + self.submit("first", 1) + batch, deadline = self.ready(trigger=2, target=2) + self.assertIsNone(batch) + self.assertIsNotNone(deadline) + self.submit("second", 2) + + reservation = self.reserve(trigger=2, target=2) + candidates = self.assistant.start_reserved_batch(reservation.operation_id) + + self.assertEqual(reservation.executor, "assistant") + self.assertEqual({item.session_id for item in reservation.participants}, {"first", "second"}) + self.assertEqual({candidate.uid for candidate in candidates}, {_uid(1), _uid(2)}) + for session in ("first", "second"): + state = self.assistant.read(session) + self.assertEqual(state.current_operation_id, reservation.operation_id) + self.assertIsNone(state.pid) + + def test_worker_never_selects_or_runs_an_assistant_reservation(self) -> None: + self.submit("assistant-session", 1) + reservation = self.reserve() + self.assistant.start_reserved_batch(reservation.operation_id) + with patch.object(self.worker, "_start_worker_if_needed"): + self.worker.submit("worker-session", "test evidence", candidate_uid=_uid(2)) + + selected, _ = self.ready(self.worker) + self.assertEqual([item.session_id for item in selected], ["worker-session"]) + callback = Mock(return_value="completed") + result = self.worker.run_pending_batches( + callback, trigger_candidates=1, target_batch_candidates=1, max_wait_seconds=0 + ) + + self.assertEqual(result.processed, 1) + self.assertNotEqual(callback.call_args.args[0], reservation.operation_id) + state = self.assistant.read("assistant-session") + self.assertEqual(state.current_operation_id, reservation.operation_id) + self.assertEqual([job.candidate_uid for job in state.current_batch], [_uid(1)]) + + def test_assistant_ready_does_not_recover_an_unrelated_dead_worker(self) -> None: + with patch.object(self.worker, "_start_worker_if_needed"): + submitted = self.worker.submit("worker-session", "test evidence", candidate_uid=_uid(1)) + self.worker._write( + "worker-session", + replace(submitted, phase="running", current_batch=submitted.pending, pending=[], pid=999999), + ) + before = self.worker._state_path("worker-session").read_bytes() + self.submit("assistant-session", 2) + + with patch.object(AsyncUpdateStore, "_worker_snapshot", side_effect=AssertionError("foreign worker inspected")): + selected, _ = self.ready() + + self.assertEqual([item.session_id for item in selected], ["assistant-session"]) + self.assertEqual(self.worker._state_path("worker-session").read_bytes(), before) + + def test_assistant_running_state_does_not_depend_on_the_invoking_process(self) -> None: + self.submit("assistant-session", 1) + reservation = self.reserve() + self.assistant.start_reserved_batch(reservation.operation_id) + fresh_process_store = AsyncUpdateStore(self.root, "update", executor="assistant") + + with patch.object(AsyncUpdateStore, "_worker_snapshot", side_effect=AssertionError("worker inspected")): + state = fresh_process_store.read("assistant-session") + default_reader_state = self.worker.read("assistant-session") + + self.assertEqual(state.status, "running") + self.assertEqual(state.phase, "running") + self.assertIsNone(state.error) + self.assertEqual(default_reader_state.current_operation_id, reservation.operation_id) + + def test_new_candidates_do_not_join_the_reserved_or_running_batch(self) -> None: + self.submit("first", 1) + self.submit("second", 2) + reservation = self.reserve(trigger=2, target=2) + self.submit("first", 3) + candidates = self.assistant.start_reserved_batch(reservation.operation_id) + self.submit("first", 4) + + self.assertEqual({candidate.uid for candidate in candidates}, {_uid(1), _uid(2)}) + state = self.assistant.read("first") + self.assertEqual([job.candidate_uid for job in state.current_batch], [_uid(1)]) + self.assertEqual([job.candidate_uid for job in state.pending], [_uid(3), _uid(4)]) + self.assistant.finish_reserved_batch(reservation.operation_id, "completed") + state = self.assistant.read("first") + + self.assertEqual(state.current_batch, []) + self.assertEqual([job.candidate_uid for job in state.pending], [_uid(3), _uid(4)]) + self.assertEqual(state.phase, "waiting") + self.assertEqual(state.executor, "assistant") + self.assertIsNone(state.pid) + self.assertIsNone(self.assistant._read_reservation(reservation.operation_id)) + + def test_active_session_cannot_switch_executor_on_submit(self) -> None: + self.submit("shared-session", 1) + + with self.assertRaises(ValueError): + self.worker.submit("shared-session", "test evidence", candidate_uid=_uid(2)) + + self.assertEqual([job.candidate_uid for job in self.assistant.read("shared-session").pending], [_uid(1)]) + self.assertIsNone(UpdateQueueStore(self.root).read_outbox(_uid(2))) + + def test_wrong_executor_cannot_reserve_start_or_finish_a_batch(self) -> None: + self.submit("assistant-session", 1) + batch, _ = self.ready() + with self.assertRaises(RuntimeError): + self.worker.reserve_batch(batch) + reservation = self.assistant.reserve_batch(batch) + self.assertIsNotNone(reservation) + with self.assertRaises(RuntimeError): + self.worker.start_reserved_batch(reservation.operation_id) + with self.assertRaises(RuntimeError): + self.worker.finish_reserved_batch(reservation.operation_id, "completed") + + def test_interrupted_outbox_submit_retains_assistant_ownership(self) -> None: + original_write = self.assistant._write + + def interrupt_after_outbox(session_id, state): + if state.pending: + raise RuntimeError("interrupted after outbox write") + original_write(session_id, state) + + with patch.object(self.assistant, "_write", side_effect=interrupt_after_outbox): + with self.assertRaises(RuntimeError): + self.submit("assistant-session", 1) + + self.assertIsNotNone(UpdateQueueStore(self.root).read_outbox(_uid(1))) + self.assertEqual(self.assistant._read_raw("assistant-session").executor, "assistant") + loaded = self.worker.read("assistant-session") + self.assertEqual(loaded.executor, "assistant") + self.assertEqual([job.candidate_uid for job in loaded.pending], [_uid(1)]) + self.assertIsNone(self.ready(self.worker)[0]) + + +def _uid(number: int) -> str: + return f"{number:032x}" + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_assistant_service.py b/tests/test_assistant_service.py new file mode 100644 index 0000000..947e7b4 --- /dev/null +++ b/tests/test_assistant_service.py @@ -0,0 +1,227 @@ +from datetime import UTC, datetime +from pathlib import Path +from unittest.mock import patch + +from rightmemory.assistant import AssistantClaimLost, AssistantService +from rightmemory.assistant_write import AssistantStaleVersionError +from rightmemory.semantic_operation import SemanticOperationStore +from rightmemory.update_record import UpdateRecordStore +from tests.isolated_write_test_base import IsolatedWriteTestBase + + +class AssistantServiceTests(IsolatedWriteTestBase): + def setUp(self): + super().setUp() + (self.root / "rightmemory.toml").write_text( + "[update.async]\ntrigger_candidates = 2\ntarget_batch_candidates = 2\nmax_wait_seconds = 1\n" + ) + self.service = AssistantService(self.root) + update_prompt = patch("rightmemory.assistant.build_assistant_update_instructions", return_value="") + retrieve_prompt = patch("rightmemory.assistant_prompt.build_assistant_retrieve_instructions", return_value="") + update_prompt.start() + retrieve_prompt.start() + self.addCleanup(update_prompt.stop) + self.addCleanup(retrieve_prompt.stop) + + def _submit_batch(self): + first = self.service.submit("conversation-a", "evidence-a", "Project fictional code is Orion.") + second = self.service.submit("conversation-b", "evidence-b", "Project code applies to the export plan.") + return first, second + + def _begin(self): + self._submit_batch() + result = self.service.begin_update() + self.assertTrue(result["claimed"]) + return result + + def _edit(self, task, line="- `project` Project fictional code is Orion. → []\n"): + worktree = Path(task["worktree"]) + self._append_memory(worktree, line) + self._git("add", "MEMORY.md", cwd=worktree) + self._git("commit", "-m", "memory: record project", cwd=worktree) + + def test_submit_batch_native_edit_complete_and_independent_context(self): + first, second = self._submit_batch() + self.assertFalse(first["update"]["ready"]) + self.assertTrue(second["update"]["ready"]) + pending = self.service.context("independent-reader") + self.assertEqual(pending["pending_count"], 2) + task = self.service.begin_update() + self.assertEqual(task["batch_id"], second["update"]["batch_id"]) + self._edit(task) + receipt = self.service.complete_update(task["task_id"], "recorded") + self.assertEqual(receipt["status"], "completed") + self.assertEqual(receipt["changed_paths"], ["MEMORY.md"]) + head = self._git("rev-parse", "HEAD") + self.assertEqual(receipt["landed_commit"], head) + retained = UpdateRecordStore(self.root).read(task["batch_id"]) + self.assertEqual(len(retained.candidates), 2) + fresh = AssistantService(self.root).context("another-reader") + self.assertEqual(fresh["pending_count"], 0) + self.assertEqual(fresh["memory_commit"], head) + self.assertIn("Orion", (self.root / "MEMORY.md").read_text()) + self.assertFalse(Path(task["worktree"]).exists()) + + def test_submit_and_completion_ids_are_immutable_after_success(self): + task = self._begin() + self._edit(task) + first = self.service.complete_update(task["task_id"], "done") + head = self._git("rev-parse", "HEAD") + duplicate = AssistantService(self.root).complete_update(task["task_id"]) + self.assertEqual(first["landed_commit"], duplicate["landed_commit"]) + self.assertEqual(self._git("rev-parse", "HEAD"), head) + saved = self.service.submit("conversation-a", "evidence-a", "Project fictional code is Orion.") + self.assertTrue(saved["duplicate"]) + self.assertEqual(self.service.context("fresh")["pending_count"], 0) + with self.assertRaises(ValueError): + self.service.submit("conversation-a", "evidence-a", "different evidence") + with self.assertRaises(ValueError): + self.service.complete_update(task["task_id"], "changed result") + + def test_repeat_begin_returns_existing_task_without_duplicate_claim(self): + first = self._begin() + second = AssistantService(self.root).begin_update() + self.assertFalse(second["claimed"]) + self.assertEqual(first["task_id"], second["task_id"]) + self.assertEqual(first["worktree"], second["worktree"]) + + def test_new_submissions_stay_pending_after_claimed_batch_finishes(self): + task = self._begin() + late = self.service.submit("conversation-a", "late", "Another finding arrived during Update.") + self.service.complete_update(task["task_id"]) + state = self.service.store.read("conversation-a") + self.assertEqual([job.candidate_uid for job in state.pending], [late["candidate_uid"]]) + self.assertEqual(self.service.context("reader")["pending_count"], 1) + + def test_all_skipped_batch_still_settles_once(self): + task = self._begin() + before = (self.root / "MEMORY.md").read_bytes() + receipt = self.service.complete_update(task["task_id"], "no durable change needed") + self.assertEqual(receipt["changed_paths"], []) + self.assertEqual((self.root / "MEMORY.md").read_bytes(), before) + self.assertEqual(self.service.context("reader")["pending_count"], 0) + self.assertIsNotNone(UpdateRecordStore(self.root).read(task["batch_id"])) + head = self._git("rev-parse", "HEAD") + self.service.complete_update(task["task_id"]) + self.assertEqual(head, self._git("rev-parse", "HEAD")) + + def test_stale_edit_retries_same_batch_with_new_attempt_and_fences_old_task(self): + task = self._begin() + self._edit(task) + self._append_memory(self.root, "- `concurrent` Concurrent evidence. → []\n") + self._git("add", "MEMORY.md") + self._git("commit", "-m", "memory: concurrent update") + with self.assertRaises(AssistantStaleVersionError): + self.service.complete_update(task["task_id"]) + newer_head = self._git("rev-parse", "HEAD") + replacement = self.service.begin_update(retry_task=task["task_id"]) + self.assertEqual(task["batch_id"], replacement["batch_id"]) + self.assertNotEqual(task["task_id"], replacement["task_id"]) + self.assertEqual(replacement["base_commit"], newer_head) + with self.assertRaises(AssistantClaimLost): + self.service.complete_update(task["task_id"]) + self._edit(replacement) + self.service.complete_update(replacement["task_id"]) + memory = (self.root / "MEMORY.md").read_text() + self.assertIn("Concurrent evidence", memory) + self.assertIn("Orion", memory) + + def test_expired_unfinished_claim_can_be_replaced_and_old_no_change_is_fenced(self): + task = self._begin() + later = datetime(2100, 1, 1, tzinfo=UTC) + with patch("rightmemory.assistant._now", return_value=later): + replacement = self.service.begin_update() + self.assertNotEqual(task["task_id"], replacement["task_id"]) + with self.assertRaises(AssistantClaimLost): + self.service.complete_update(task["task_id"]) + self.service.complete_update(replacement["task_id"]) + + def test_validation_failure_can_be_corrected_before_completion(self): + task = self._begin() + self._edit(task, "- `bad` Broken edge. → [does-not-exist]\n") + with self.assertRaises(RuntimeError): + self.service.complete_update(task["task_id"]) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + worktree = Path(task["worktree"]) + text = (worktree / "MEMORY.md").read_text().replace("→ [does-not-exist]", "→ []") + (worktree / "MEMORY.md").write_text(text) + self._git("add", "MEMORY.md", cwd=worktree) + self._git("commit", "--amend", "--no-edit", cwd=worktree) + receipt = self.service.complete_update(task["task_id"]) + self.assertEqual(receipt["status"], "completed") + + def test_illegal_committed_path_never_lands(self): + task = self._begin() + worktree = Path(task["worktree"]) + (worktree / "outside.txt").write_text("unexpected") + self._git("add", "outside.txt", cwd=worktree) + self._git("commit", "-m", "illegal candidate", cwd=worktree) + with self.assertRaises(RuntimeError): + self.service.complete_update(task["task_id"]) + self.assertFalse((self.root / "outside.txt").exists()) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + def test_interrupted_workspace_preparation_resumes_same_task(self): + self._submit_batch() + with patch.object(self.service.writer, "prepare", side_effect=OSError("interrupted")): + with self.assertRaises(OSError): + self.service.begin_update() + task_id = self.service.update_status()["task_id"] + resumed = AssistantService(self.root).begin_update() + self.assertEqual(resumed["task_id"], task_id) + self.assertTrue(Path(resumed["worktree"]).is_dir()) + self.service.complete_update(task_id) + + def test_interrupted_after_publication_before_queue_ack_recovers_without_second_commit(self): + task = self._begin() + self._edit(task) + with patch.object(self.service, "_finish", side_effect=OSError("interrupted before acknowledgement")): + with self.assertRaises(OSError): + self.service.complete_update(task["task_id"], "done") + head = self._git("rev-parse", "HEAD") + operation = SemanticOperationStore(self.root).read(self.service.writer.read(task["task_id"]).operation_id) + self.assertEqual(operation.phase, "committed") + status = AssistantService(self.root).update_status(task["task_id"]) + self.assertEqual(status["landed_commit"], head) + self.assertEqual(self._git("rev-parse", "HEAD"), head) + self.assertEqual(self.service.context("reader")["pending_count"], 0) + + def test_interrupted_after_partial_ack_recovers_remaining_participants(self): + task = self._begin() + original = self.service.store._write + + def fail_one(session_id, state): + if session_id == "conversation-b" and state.status == "succeeded": + raise OSError("interrupted during acknowledgement") + return original(session_id, state) + + with patch.object(self.service.store, "_write", side_effect=fail_one): + with self.assertRaises(OSError): + self.service.complete_update(task["task_id"]) + head = self._git("rev-parse", "HEAD") + self.service.complete_update(task["task_id"]) + self.assertEqual(self._git("rev-parse", "HEAD"), head) + self.assertEqual(self.service.context("reader")["pending_count"], 0) + + def test_rejects_path_like_task_id_and_runtime_symlink(self): + with self.assertRaises(ValueError): + self.service.complete_update("../../escape") + external = self.root / "external" + external.mkdir() + runtime = self.root / ".runtime" + runtime.mkdir(exist_ok=True) + (runtime / "assistant").rename(runtime / "assistant-saved") + (runtime / "assistant").symlink_to(external, target_is_directory=True) + with self.assertRaises(ValueError): + AssistantService(self.root) + self.assertEqual(list(external.iterdir()), []) + + def test_interrupted_submit_keeps_exact_request_identity(self): + with patch.object(self.service.store, "submit", side_effect=OSError("interrupted")): + with self.assertRaises(OSError): + self.service.submit("conversation", "request", "exact evidence") + saved = AssistantService(self.root).submit("conversation", "request", "exact evidence") + self.assertTrue(saved["saved"]) + with self.assertRaises(ValueError): + self.service.submit("conversation", "request", "different evidence") + self.assertEqual(len(self.service.store.read("conversation").pending), 1) diff --git a/tests/test_assistant_service_review.py b/tests/test_assistant_service_review.py new file mode 100644 index 0000000..283b988 --- /dev/null +++ b/tests/test_assistant_service_review.py @@ -0,0 +1,235 @@ +from __future__ import annotations + +from datetime import datetime, timedelta +from unittest.mock import patch + +from rightmemory.assistant import AssistantService +from rightmemory.semantic_operation import SemanticOperationStore +from rightmemory.update_queue import UpdateQueueStore +from rightmemory.update_queue_git import QUEUE_FINALIZER +from tests.isolated_write_test_base import IsolatedWriteTestBase +from tests.update_queue_git_base import GitUpdateQueueTestBase + + +class AssistantServiceRecoveryReviewTests(IsolatedWriteTestBase): + def setUp(self): + super().setUp() + instructions = patch("rightmemory.assistant.build_assistant_update_instructions", return_value="test") + instructions.start() + self.addCleanup(instructions.stop) + self.service = self.new_service() + + def new_service(self): + service = AssistantService(self.root) + service.batch_options = { + "trigger_candidates": 1, "target_batch_candidates": 1, "max_wait_seconds": 0, + } + return service + + def begin(self): + self.service.submit("first", "request-one", "test evidence") + return self.service.begin_update() + + def commit_memory(self, task_id): + task = self.service.writer.read(task_id) + self._append_memory(task.worktree, "- `two` accepted fact → []\n") + self._git("add", "MEMORY.md", cwd=task.worktree) + self._git("commit", "-m", "memory: review fixture", cwd=task.worktree) + + def test_terminal_write_receipt_recovers_queue_ack_without_a_second_commit(self): + begin = self.begin() + task_id = begin["task_id"] + self.commit_memory(task_id) + with patch.object(self.service.store, "finish_reserved_batch", side_effect=OSError("ack interrupted")): + with self.assertRaises(OSError): + self.service.complete_update(task_id) + landed = self._git("rev-parse", "HEAD") + self.assertNotEqual(landed, self.initial_head) + + recovered = self.new_service().update_status(task_id) + + self.assertEqual(recovered["status"], "completed") + self.assertEqual(recovered["landed_commit"], landed) + self.assertEqual(self._git("rev-parse", "HEAD"), landed) + self.assertEqual(self.service.store.read("first").current_batch, []) + self.assertIsNone(self.service.store._read_reservation(begin["batch_id"])) + + def test_partial_cross_session_ack_recovers_the_remaining_participant(self): + self.service.submit("first", "request-one", "test evidence") + self.service.submit("second", "request-two", "test evidence") + self.service.batch_options.update(trigger_candidates=2, target_batch_candidates=2) + begin = self.service.begin_update() + original_write = self.service.store._write + + def interrupt_second_ack(session_id, state): + if session_id == "second" and state.last_operation_id == begin["batch_id"]: + raise OSError("second ack interrupted") + original_write(session_id, state) + + with patch.object(self.service.store, "_write", side_effect=interrupt_second_ack): + with self.assertRaises(OSError): + self.service.complete_update(begin["task_id"]) + landed = self._git("rev-parse", "HEAD") + self.assertEqual(self.service.store._read_raw("first").current_batch, []) + self.assertEqual(len(self.service.store._read_raw("second").current_batch), 1) + + recovered = self.new_service().update_status(begin["task_id"]) + + self.assertEqual(recovered["status"], "completed") + self.assertEqual(self._git("rev-parse", "HEAD"), landed) + self.assertEqual(self.service.store.read("second").current_batch, []) + self.assertEqual(UpdateQueueStore(self.root).outbox_candidates(), ()) + + def test_retry_recovers_prepared_result_before_allocating_a_new_task(self): + begin = self.begin() + task_id = begin["task_id"] + self.commit_memory(task_id) + with patch.object(self.service.writer.supervisor, "publish_external", side_effect=OSError("publish interrupted")): + with self.assertRaises(OSError): + self.service.complete_update(task_id) + task = self.service.writer.read(task_id) + self.assertEqual(SemanticOperationStore(self.root).read(task.operation_id).phase, "prepared") + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + recovered = self.new_service().begin_update(retry_task=task_id) + + self.assertEqual(recovered["status"], "completed") + self.assertEqual(recovered["task_id"], task_id) + self.assertEqual(self._git("rev-list", "--count", f"{self.initial_head}..HEAD"), "1") + + + def test_retry_recovers_landed_commit_when_semantic_receipt_was_interrupted(self): + begin = self.begin() + task_id = begin["task_id"] + self.commit_memory(task_id) + task = self.service.writer.read(task_id) + complete_commit = SemanticOperationStore.complete_commit + + def interrupt_receipt(store, operation_id, commit): + if operation_id == task.operation_id: + raise OSError("receipt interrupted") + return complete_commit(store, operation_id, commit) + + with patch.object(SemanticOperationStore, "complete_commit", interrupt_receipt): + with self.assertRaises(OSError): + self.service.complete_update(task_id) + landed = self._git("rev-parse", "HEAD") + self.assertNotEqual(landed, self.initial_head) + self.assertEqual(SemanticOperationStore(self.root).read(task.operation_id).phase, "prepared") + + recovered = self.new_service().begin_update(retry_task=task_id) + + self.assertEqual(recovered["status"], "completed") + self.assertEqual(recovered["landed_commit"], landed) + self.assertEqual(self._git("rev-parse", "HEAD"), landed) + + def test_repeated_submit_after_completion_does_not_enqueue_or_commit_again(self): + begin = self.begin() + first = self.service.complete_update(begin["task_id"]) + service = self.new_service() + + submission = service.submit("first", "request-one", "test evidence") + repeated = service.complete_update(begin["task_id"]) + + self.assertTrue(submission["duplicate"]) + self.assertFalse(submission["update"]["ready"]) + self.assertEqual(service.store.read("first").pending, []) + self.assertEqual(repeated["landed_commit"], first["landed_commit"]) + self.assertEqual(self._git("rev-list", "--count", f"{self.initial_head}..HEAD"), "1") + + +class AssistantForeignCompletionReviewTests(GitUpdateQueueTestBase): + def setUp(self): + super().setUp() + instructions = patch("rightmemory.assistant.build_assistant_update_instructions", return_value="test") + instructions.start() + self.addCleanup(instructions.stop) + for root in (self.first, self.second): + (root / "rightmemory.toml").write_text("[sync]\nenabled = true\n", encoding="utf-8") + self.first_service = self.service(self.first) + self.second_service = self.service(self.second) + + def service(self, root): + service = AssistantService(root) + service.batch_options = { + "trigger_candidates": 1, "target_batch_candidates": 1, "max_wait_seconds": 0, + } + return service + + def complete_on_another_device(self, *, prepare_first=True): + self.first_service.submit("first", "request-one", "test evidence") + first = self.first_service.begin_update() + self.assertEqual(first["status"], "ready") + if prepare_first: + result = self.first_service.writer.complete( + first["task_id"], external_finalizer=QUEUE_FINALIZER, + check_current=lambda: self.first_service._check_current(first["task_id"]), + ) + self.assertTrue(result.prepared) + future = datetime.fromisoformat(first["expires_at"]) + timedelta(seconds=1) + clock = patch("rightmemory.assistant._now", return_value=future) + clock.start() + self.addCleanup(clock.stop) + queue_clock = patch("rightmemory.update_queue_git._utc_now", return_value=future) + queue_clock.start() + self.addCleanup(queue_clock.stop) + second = self.second_service.begin_update() + self.assertEqual(second["status"], "ready") + self.assertEqual(second["batch_id"], first["batch_id"]) + self.assertNotEqual(second["task_id"], first["task_id"]) + completed = self.second_service.complete_update(second["task_id"]) + self.assertEqual(completed["status"], "completed") + return first, completed + + def test_foreign_token_completion_releases_old_active_and_allows_the_next_batch(self): + first, foreign = self.complete_on_another_device() + + recovered = self.first_service.begin_update(retry_task=first["task_id"]) + + self.assertEqual(recovered["status"], "completed_elsewhere") + self.assertEqual(recovered["landed_commit"], foreign["landed_commit"]) + self.assertIsNone(self.first_service._active()) + self.first_service.submit("first", "request-two", "new evidence") + following = self.first_service.begin_update() + self.assertEqual(following["status"], "ready") + self.assertNotEqual(following["batch_id"], first["batch_id"]) + + def test_foreign_terminal_receipt_survives_interruption_before_local_ack(self): + first, foreign = self.complete_on_another_device() + with patch.object( + self.first_service.store, "acknowledge_synchronized", side_effect=OSError("ack interrupted") + ): + with self.assertRaises(OSError): + self.first_service.begin_update(retry_task=first["task_id"]) + + recovered = self.service(self.first).update_status(first["task_id"]) + + self.assertEqual(recovered["status"], "completed_elsewhere") + self.assertEqual(recovered["landed_commit"], foreign["landed_commit"]) + self.assertIsNone(self.first_service._active()) + + def test_foreign_running_operation_recovers_without_reopening_dirty_native_edits(self): + first, foreign = self.complete_on_another_device(prepare_first=False) + task_id = first["task_id"] + with patch.object( + self.first_service.writer.supervisor, "_validate_candidate", side_effect=RuntimeError("validation rejected") + ): + with self.assertRaises(RuntimeError): + self.first_service.writer.complete(task_id, external_finalizer=QUEUE_FINALIZER) + task = self.first_service.writer.read(task_id) + self.assertIsNone(task.source_commit) + self.assertEqual(SemanticOperationStore(self.first).read(task.operation_id).phase, "running") + (task.worktree / "MEMORY.md").write_text("# Unfinished native correction\n", encoding="utf-8") + with patch.object( + self.first_service.store, "acknowledge_synchronized", side_effect=OSError("ack interrupted") + ): + with self.assertRaises(OSError): + self.first_service.begin_update(retry_task=task_id) + + recovered = self.service(self.first).update_status(task_id) + + self.assertEqual(recovered["status"], "completed_elsewhere") + self.assertEqual(recovered["landed_commit"], foreign["landed_commit"]) + self.assertTrue(task.worktree.is_dir()) + self.assertTrue(self._git(task.worktree, "status", "--porcelain")) + self.assertIsNone(self.first_service._active()) diff --git a/tests/test_assistant_sync.py b/tests/test_assistant_sync.py new file mode 100644 index 0000000..9fdfce9 --- /dev/null +++ b/tests/test_assistant_sync.py @@ -0,0 +1,231 @@ +from __future__ import annotations + +from datetime import datetime, timedelta +from pathlib import Path +from unittest.mock import patch + +from rightmemory.assistant import AssistantService +from rightmemory.semantic_operation import SemanticOperationStore +from rightmemory.sync import SyncManager +from rightmemory.update_queue import UpdateQueueStore +from rightmemory.update_queue_git import ( + GitUpdateQueueCoordinator, + UpdateQueueLeaseLost, + UpdateQueueUnavailable, +) +from rightmemory.update_record import UpdateRecordStore +from tests.update_queue_git_base import GitUpdateQueueTestBase + + +class AssistantSyncTests(GitUpdateQueueTestBase): + def setUp(self): + super().setUp() + for root in (self.first, self.second): + (root / "rightmemory.toml").write_text( + "[sync]\nenabled = true\n\n[update.async]\n" + "trigger_candidates = 1\ntarget_batch_candidates = 1\n", + encoding="utf-8", + ) + exclude = root / ".git" / "info" / "exclude" + exclude.parent.mkdir(parents=True, exist_ok=True) + with exclude.open("a", encoding="utf-8") as handle: + handle.write("\nrightmemory.toml\n") + for target in ( + "rightmemory.assistant.build_assistant_update_instructions", + "rightmemory.assistant_prompt.build_assistant_retrieve_instructions", + ): + instructions = patch(target, return_value="test") + instructions.start() + self.addCleanup(instructions.stop) + self.service = AssistantService(self.first) + self.other = AssistantService(self.second) + + def _begin(self): + submission = self.service.submit("first-session", "request-one", "retain the project constraint") + self.assertTrue(submission["saved"]) + self.assertTrue(submission["update"]["ready"]) + begin = self.service.begin_update() + self.assertEqual(begin["status"], "ready") + return submission, begin + + def _commit_memory(self, begin, *, key="two", value="native assistant update"): + worktree = Path(begin["worktree"]) + with (worktree / "MEMORY.md").open("a", encoding="utf-8") as handle: + handle.write(f"- `{key}` {value} → []\n") + self._git(worktree, "add", "MEMORY.md") + self._git(worktree, "commit", "-m", "memory: native assistant edit") + + def _operation(self, begin): + task = self.service.writer.read(begin["task_id"]) + return SemanticOperationStore(self.first).read(task.operation_id) + + def _remote_head(self): + return self._git(self.remote, "rev-parse", "refs/heads/main") + + def test_shared_submission_native_update_and_other_clone_retrieve(self): + submission, begin = self._begin() + active = self.service.begin_update() + self.assertEqual(active["task_id"], begin["task_id"]) + self.assertFalse(active["claimed"]) + self._commit_memory(begin) + + completed = self.service.complete_update(begin["task_id"], output="retained the constraint") + retrieved = self.other.context("independent-retrieve") + + self.assertEqual(completed["status"], "completed") + self.assertEqual(completed["sync"], "published") + self.assertEqual(completed["changed_paths"], ["MEMORY.md"]) + self.assertEqual(retrieved["memory_commit"], completed["landed_commit"]) + self.assertEqual(retrieved["pending_count"], 0) + self.assertIn(str(self.second / "MEMORY.md"), retrieved["source_files"]) + self.assertIn("native assistant update", (self.second / "MEMORY.md").read_text()) + record = UpdateRecordStore(self.second).read(begin["batch_id"]) + self.assertEqual([item.uid for item in record.candidates], [submission["candidate_uid"]]) + snapshot = UpdateQueueStore(self.second).snapshot() + self.assertEqual(snapshot.candidates, ()) + self.assertIsNone(snapshot.lease) + self.assertFalse(Path(begin["worktree"]).exists()) + remote_head = self._remote_head() + repeated = AssistantService(self.first).complete_update(begin["task_id"]) + self.assertEqual(repeated["landed_commit"], completed["landed_commit"]) + self.assertEqual(self._remote_head(), remote_head) + + def test_default_worker_on_same_root_cannot_reclaim_assistant_active_lease(self): + _submission, begin = self._begin() + worker = GitUpdateQueueCoordinator(self.service.sync_config) + + worker_claim = worker.claim_next(**self.service.batch_options) + + self.assertIsNone(worker_claim.claim) + self.assertIsNotNone(worker_claim.next_attempt_at) + native_claim = AssistantService(self.first)._coordinator().claim_next(**self.service.batch_options) + self.assertIsNotNone(native_claim.claim) + self.assertEqual(native_claim.claim.batch_id, begin["batch_id"]) + repeated = AssistantService(self.first).begin_update() + self.assertEqual(repeated["task_id"], begin["task_id"]) + self.assertFalse(repeated["claimed"]) + + def test_failed_publication_preserves_prepared_task_and_retry_needs_no_native_workspace(self): + _submission, begin = self._begin() + self._commit_memory(begin) + start_head = self._git(self.first, "rev-parse", "HEAD") + remote_head = self._remote_head() + with patch.object(GitUpdateQueueCoordinator, "_push_cas", return_value=False): + with self.assertRaises(UpdateQueueUnavailable): + self.service.complete_update(begin["task_id"], output="fixed completion") + prepared = self._operation(begin) + self.assertEqual(prepared.phase, "prepared") + with self.assertRaises(UpdateQueueUnavailable): + AssistantService(self.first).begin_update(retry_task=begin["task_id"]) + self.assertEqual(self._git(self.first, "rev-parse", "HEAD"), start_head) + self.assertEqual(self._remote_head(), remote_head) + self.assertEqual(self.service.update_status()["task_id"], begin["task_id"]) + self._git(self.first, "worktree", "remove", begin["worktree"]) + + retried = AssistantService(self.first).begin_update(retry_task=begin["task_id"]) + + self.assertEqual(retried["status"], "completed") + self.assertEqual(retried["task_id"], begin["task_id"]) + self.assertEqual(retried["output"], "fixed completion") + self.assertEqual(self._operation(begin).outcome.metadata["candidate_commit"], + prepared.outcome.metadata["candidate_commit"]) + self.assertEqual((self.first / "MEMORY.md").read_text().count("native assistant update"), 1) + landed_operations = self._git( + self.first, "log", "--format=%H", "--fixed-strings", + f"--grep=RightMemory-Operation: {prepared.operation_id}", "HEAD", + ).splitlines() + self.assertEqual(len(landed_operations), 1) + self.assertEqual(self._remote_head(), retried["landed_commit"]) + + def test_published_submission_stays_local_until_its_queue_state_is_admitted(self): + initial_head = self._git(self.first, "rev-parse", "HEAD") + with patch.object(SyncManager, "_publish_candidate", side_effect=OSError("admission interrupted")): + submitted = self.service.submit("first-session", "request-one", "retain pending evidence") + self.assertTrue(submitted["saved"]) + self.assertEqual(submitted["queue"]["status"], "pending") + self.assertEqual(self._git(self.first, "rev-parse", "HEAD"), initial_head) + self.assertEqual( + [item.uid for item in UpdateQueueStore(self.first).outbox_candidates()], + [submitted["candidate_uid"]], + ) + remote_head = self._remote_head() + self.assertNotEqual(remote_head, initial_head) + + retrieved = AssistantService(self.first).context("pending-retrieve") + + self.assertEqual(retrieved["pending_count"], 1) + self.assertTrue(retrieved["update"]["ready"]) + self.assertEqual(UpdateQueueStore(self.first).outbox_candidates(), ()) + self.assertEqual(self._remote_head(), remote_head) + + def test_shared_landing_with_lost_receipt_recovers_without_a_second_publication(self): + _submission, begin = self._begin() + self._commit_memory(begin) + with patch.object(self.service.writer, "complete_external", side_effect=OSError("receipt interrupted")): + with self.assertRaisesRegex(OSError, "receipt interrupted"): + self.service.complete_update(begin["task_id"], output="published once") + remote_head = self._remote_head() + self.assertEqual(self._git(self.first, "rev-parse", "HEAD"), remote_head) + self.assertEqual(self._operation(begin).phase, "prepared") + self._git(self.first, "worktree", "remove", begin["worktree"]) + + recovered = AssistantService(self.first).begin_update(retry_task=begin["task_id"]) + + self.assertEqual(recovered["status"], "completed") + self.assertEqual(recovered["landed_commit"], remote_head) + self.assertEqual(recovered["output"], "published once") + self.assertEqual(recovered["changed_paths"], ["MEMORY.md"]) + self.assertEqual(self._remote_head(), remote_head) + self.assertEqual(self._operation(begin).phase, "committed") + self.assertEqual((self.first / "MEMORY.md").read_text().count("native assistant update"), 1) + + def test_remote_lease_takeover_fences_old_native_task(self): + _submission, old = self._begin() + self._commit_memory(old, key="old-owner", value="obsolete attempt") + self.assertEqual(self.other.begin_update()["status"], "not_ready") + expires = datetime.fromisoformat(old["expires_at"]) + timedelta(seconds=1) + with ( + patch("rightmemory.update_queue_git._utc_now", return_value=expires), + patch("rightmemory.assistant._now", return_value=expires), + ): + replacement = self.other.begin_update() + self.assertEqual(replacement["status"], "ready") + self.assertEqual(replacement["batch_id"], old["batch_id"]) + self.assertNotEqual(replacement["task_id"], old["task_id"]) + remote_head = self._remote_head() + with self.assertRaises(UpdateQueueLeaseLost): + self.service.complete_update(old["task_id"]) + self.assertEqual(self._remote_head(), remote_head) + self.assertNotIn("obsolete attempt", self._git(self.remote, "show", "main:MEMORY.md")) + + self._commit_memory(replacement, key="new-owner", value="current owner result") + completed = self.other.complete_update(replacement["task_id"]) + + self.assertEqual(completed["status"], "completed") + obsolete = self.service.complete_update(old["task_id"]) + self.assertEqual(obsolete["status"], "completed_elsewhere") + self.assertEqual(obsolete["changed_paths"], []) + self.assertEqual(self._remote_head(), completed["landed_commit"]) + memory = self._git(self.remote, "show", "main:MEMORY.md") + self.assertIn("current owner result", memory) + self.assertNotIn("obsolete attempt", memory) + + def test_ordinary_pull_conflict_stops_update_without_internal_model_repair(self): + for root, value in ((self.first, "local fact"), (self.second, "remote fact")): + (root / "MEMORY.md").write_text(f"# Domain\n\n- `one` {value} → []\n", encoding="utf-8") + self._git(root, "add", "MEMORY.md") + self._git(root, "commit", "-m", "memory: concurrent fact") + self._git(self.second, "push") + before_head = self._git(self.first, "rev-parse", "HEAD") + before_memory = (self.first / "MEMORY.md").read_bytes() + with patch("rightmemory.runtime.RightMemoryRuntime", side_effect=AssertionError("internal runtime started")): + begin = self.service.begin_update() + synced = self.service.sync("pull") + + self.assertEqual(begin["status"], "sync_required") + self.assertEqual(begin["sync"]["status"], "conflict") + self.assertEqual(synced["sync"]["status"], "conflict") + self.assertEqual(self._git(self.first, "rev-parse", "HEAD"), before_head) + self.assertEqual((self.first / "MEMORY.md").read_bytes(), before_memory) + self.assertEqual(self._git(self.first, "status", "--porcelain"), "") + self.assertEqual(SemanticOperationStore(self.first).list_records(), ()) diff --git a/tests/test_assistant_write.py b/tests/test_assistant_write.py new file mode 100644 index 0000000..2d0bf37 --- /dev/null +++ b/tests/test_assistant_write.py @@ -0,0 +1,308 @@ +import json +import os +import unittest +from unittest.mock import patch + +from rightmemory.assistant_write import AssistantStaleVersionError, AssistantWriteManager +from rightmemory.isolated_write import IsolatedWriteSupervisor +from rightmemory.semantic_operation import SemanticOperationStore +from rightmemory.update_queue import UpdateCandidate, update_candidate_batch_id +from rightmemory.update_record import UpdateRecordStore +from tests.isolated_write_test_base import IsolatedWriteTestBase + + +class AssistantWriteTests(IsolatedWriteTestBase): + def setUp(self): + super().setUp() + self.manager = AssistantWriteManager(self.root) + self.candidates = ( + UpdateCandidate( + uid="a" * 32, + session_id="assistant-session", + display_id=1, + message="Retain the user's new project constraint.", + submitted_at="2026-10-23T10:00:00+00:00", + ), + ) + self.batch_id = update_candidate_batch_id(self.candidates) + + def _prepare(self, task_id="task-one"): + return self.manager.prepare(task_id, self.batch_id, self.candidates) + + def _commit_memory(self, task, text="- `two` native assistant edit → []\n"): + self._append_memory(task.worktree, text) + self._git("add", "MEMORY.md", cwd=task.worktree) + self._git("commit", "-m", "memory: retain assistant update", cwd=task.worktree) + return self._git("rev-parse", "HEAD", cwd=task.worktree) + + def test_native_worktree_survives_new_manager_and_finishes_with_batch_evidence(self): + task = self._prepare() + self._commit_memory(task) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + manager = AssistantWriteManager(self.root) + repeated = manager.prepare(task.task_id, self.batch_id, self.candidates) + self.assertEqual(repeated, task) + result = manager.complete(task.task_id, output="Retained the new constraint.") + + self.assertIn("native assistant edit", (self.root / "MEMORY.md").read_text()) + self.assertEqual(result.commits_landed, 1) + self.assertEqual(result.changed_paths, ("MEMORY.md",)) + self.assertEqual(result.operation_id, task.operation_id) + evidence = UpdateRecordStore(self.root).read(self.batch_id) + self.assertEqual(evidence.candidates, self.candidates) + self.assertNotEqual(evidence.operation_id, result.operation_id) + self.assertTrue(task.worktree.is_dir()) + + def test_repeated_completion_returns_original_receipt_after_new_writes_and_cleanup(self): + task = self._prepare() + self._commit_memory(task) + first = self.manager.complete(task.task_id, output="original report") + self.manager.cleanup(task.task_id) + self._append_memory(self.root, "- `later` later independent edit → []\n") + self._git("add", "MEMORY.md") + self._git("commit", "-m", "memory: later edit") + head = self._git("rev-parse", "HEAD") + + second = AssistantWriteManager(self.root).complete(task.task_id) + self.assertEqual(second.output, "original report") + self.assertEqual(second.landed_commit, first.landed_commit) + self.assertEqual(self._git("rev-parse", "HEAD"), head) + self.assertFalse(task.worktree.exists()) + with self.assertRaisesRegex(ValueError, "different output"): + self.manager.complete(task.task_id, output="a different report") + + def test_no_semantic_change_still_retains_evidence_once(self): + task = self._prepare() + original = (self.root / "MEMORY.md").read_bytes() + first = self.manager.complete(task.task_id, output="No useful changes.") + second = self.manager.complete(task.task_id) + self.assertEqual(first.changed_paths, ()) + self.assertEqual(first.landed_commit, second.landed_commit) + self.assertEqual((self.root / "MEMORY.md").read_bytes(), original) + self.assertIsNotNone(UpdateRecordStore(self.root).read(self.batch_id)) + self.assertEqual(self._git("rev-list", "--count", f"{self.initial_head}..HEAD"), "1") + + def test_prepare_rejects_same_task_for_different_batch(self): + self._prepare() + other = (UpdateCandidate(**{**self.candidates[0].__dict__, "uid": "b" * 32}),) + with self.assertRaisesRegex(ValueError, "different candidate evidence"): + self.manager.prepare("task-one", update_candidate_batch_id(other), other) + + def test_dirty_native_worktree_is_not_submitted(self): + task = self._prepare() + self._append_memory(task.worktree, "- `two` still editing → []\n") + with self.assertRaisesRegex(RuntimeError, "uncommitted changes"): + self.manager.complete(task.task_id) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + self.assertIsNone(self.manager.read(task.task_id).source_commit) + + def test_untracked_native_files_are_not_silently_discarded(self): + task = self._prepare() + (task.worktree / "notes.txt").write_text("unfinished scratch work") + with self.assertRaisesRegex(RuntimeError, "uncommitted changes"): + self.manager.complete(task.task_id) + self.assertTrue((task.worktree / "notes.txt").exists()) + + def test_illegal_path_is_rejected_even_if_later_reverted(self): + task = self._prepare() + (task.worktree / "outside-policy.txt").write_text("forbidden") + self._git("add", "outside-policy.txt", cwd=task.worktree) + self._git("commit", "-m", "invalid intermediate path", cwd=task.worktree) + self._git("rm", "outside-policy.txt", cwd=task.worktree) + self._git("commit", "-m", "remove invalid path", cwd=task.worktree) + with self.assertRaisesRegex(RuntimeError, "non-memory paths"): + self.manager.complete(task.task_id) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + @unittest.skipIf(os.name == "nt", "symlinks require Windows privileges") + def test_symlink_candidate_does_not_land(self): + task = self._prepare() + target = self.root / "private.txt" + target.write_text("unchanged") + (task.worktree / "MEMORY_link.md").symlink_to(target) + self._git("add", "MEMORY_link.md", cwd=task.worktree) + self._git("commit", "-m", "memory: invalid symlink", cwd=task.worktree) + with self.assertRaisesRegex(RuntimeError, "not a regular file"): + self.manager.complete(task.task_id) + self.assertEqual(target.read_text(), "unchanged") + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + def test_invalid_graph_can_be_corrected_in_the_same_task(self): + task = self._prepare() + self._commit_memory(task, "- `one` duplicate id → []\n") + with self.assertRaisesRegex(RuntimeError, "validation failed"): + self.manager.complete(task.task_id, output="invalid attempt") + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + self.assertIsNone(self.manager.read(task.task_id).source_commit) + + self._git("reset", "--hard", task.base_commit, cwd=task.worktree) + self._commit_memory(task) + result = self.manager.complete(task.task_id, output="corrected attempt") + self.assertEqual(result.output, "corrected attempt") + self.assertIn("native assistant edit", (self.root / "MEMORY.md").read_text()) + + def test_stale_semantic_base_does_not_overwrite_new_memory(self): + task = self._prepare() + self._commit_memory(task) + self._append_memory(self.root, "- `fresh` concurrent memory → []\n") + self._git("add", "MEMORY.md") + self._git("commit", "-m", "memory: concurrent change") + head = self._git("rev-parse", "HEAD") + with self.assertRaisesRegex(AssistantStaleVersionError, "memory changed since assistant task began"): + self.manager.complete(task.task_id) + self.assertEqual(self._git("rev-parse", "HEAD"), head) + self.assertNotIn("native assistant edit", (self.root / "MEMORY.md").read_text()) + + def test_prepared_stale_result_has_distinct_conflict_error(self): + task = self._prepare() + self._commit_memory(task) + with patch.object(IsolatedWriteSupervisor, "_land_operation_commit", side_effect=OSError("interrupted")): + with self.assertRaisesRegex(OSError, "interrupted"): + self.manager.complete(task.task_id) + self._append_memory(self.root, "- `fresh` newer memory → []\n") + self._git("add", "MEMORY.md") + self._git("commit", "-m", "memory: changed before retry") + head = self._git("rev-parse", "HEAD") + + with self.assertRaisesRegex(AssistantStaleVersionError, "before prepared memory write could land"): + self.manager.complete(task.task_id) + self.assertEqual(self._git("rev-parse", "HEAD"), head) + self.assertEqual(SemanticOperationStore(self.root).read(task.operation_id).phase, "prepared") + + def test_lost_ownership_before_preparation_does_not_publish(self): + task = self._prepare() + self._commit_memory(task) + + def reject(): + raise RuntimeError("claim was replaced") + + with self.assertRaisesRegex(RuntimeError, "claim was replaced"): + self.manager.complete(task.task_id, check_current=reject) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + receipt = SemanticOperationStore(self.root).read(task.operation_id) + self.assertEqual(receipt.phase, "running") + + def test_lost_ownership_before_publication_is_not_bypassed_by_legacy_recovery(self): + task = self._prepare() + self._commit_memory(task) + calls = 0 + + def check(): + nonlocal calls + calls += 1 + if calls == 2: + raise RuntimeError("claim was replaced") + + with self.assertRaisesRegex(RuntimeError, "claim was replaced"): + self.manager.complete(task.task_id, check_current=check) + self.assertEqual(calls, 2) + receipt = SemanticOperationStore(self.root).read(task.operation_id) + self.assertEqual(receipt.phase, "prepared") + IsolatedWriteSupervisor(self.root, "update").recover_prepared() + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + + result = self.manager.complete(task.task_id, check_current=lambda: None) + self.assertEqual(result.commits_landed, 1) + + def test_prepared_completion_recovers_after_landing_interruption_and_gc(self): + task = self._prepare() + self._commit_memory(task) + with patch.object(IsolatedWriteSupervisor, "_land_operation_commit", side_effect=OSError("interrupted")): + with self.assertRaisesRegex(OSError, "interrupted"): + self.manager.complete(task.task_id, output="original completion") + self._git("gc", "--prune=now") + result = AssistantWriteManager(self.root).complete(task.task_id) + self.assertEqual(result.output, "original completion") + self.assertEqual(result.commits_landed, 1) + self.assertEqual(self._git("rev-list", "--count", f"{self.initial_head}..HEAD"), "1") + + def test_prepared_retry_publishes_frozen_commit_without_reopening_native_workspace(self): + task = self._prepare() + self._commit_memory(task) + with patch.object(IsolatedWriteSupervisor, "_land_operation_commit", side_effect=OSError("interrupted")): + with self.assertRaisesRegex(OSError, "interrupted"): + self.manager.complete(task.task_id) + self._commit_memory(task, "- `late` changes after completion was submitted → []\n") + self._git("worktree", "remove", str(task.worktree)) + + result = AssistantWriteManager(self.root).complete(task.task_id) + self.assertEqual(result.commits_landed, 1) + memory = (self.root / "MEMORY.md").read_text() + self.assertIn("native assistant edit", memory) + self.assertNotIn("changes after completion was submitted", memory) + + def test_landed_completion_recovers_when_receipt_write_is_interrupted(self): + task = self._prepare() + self._commit_memory(task) + original = SemanticOperationStore.complete_commit + + def fail_receipt(store, operation_id, commit): + if operation_id == task.operation_id: + raise OSError("receipt interrupted") + return original(store, operation_id, commit) + + with patch.object(SemanticOperationStore, "complete_commit", fail_receipt): + with self.assertRaisesRegex(OSError, "receipt interrupted"): + self.manager.complete(task.task_id, output="written once") + head = self._git("rev-parse", "HEAD") + self._append_memory(self.root, "- `later` memory after landed outcome → []\n") + self._git("add", "MEMORY.md") + self._git("commit", "-m", "memory: advance after lost receipt") + latest_head = self._git("rev-parse", "HEAD") + result = self.manager.complete(task.task_id) + self.assertEqual(result.landed_commit, head) + self.assertEqual(self._git("rev-parse", "HEAD"), latest_head) + self.assertEqual((self.root / "MEMORY.md").read_text().count("native assistant edit"), 1) + + def test_external_publication_requires_explicit_completion(self): + task = self._prepare() + self._commit_memory(task) + prepared = self.manager.complete(task.task_id, external_finalizer="queue:token", output="prepared") + self.assertTrue(prepared.prepared) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + with self.assertRaisesRegex(ValueError, "different finalizer"): + self.manager.complete(task.task_id) + self._git("merge", "--ff-only", prepared.candidate_commit) + result = self.manager.complete_external( + task.task_id, external_finalizer="queue:token", landed_commit=prepared.candidate_commit + ) + self.assertFalse(result.prepared) + self.assertEqual(result.landed_commit, prepared.candidate_commit) + + def test_cleanup_retains_work_added_after_completion(self): + task = self._prepare() + self._commit_memory(task) + self.manager.complete(task.task_id) + (task.worktree / "notes.txt").write_text("do not discard") + with self.assertRaisesRegex(RuntimeError, "retained for review"): + self.manager.cleanup(task.task_id) + self.assertTrue((task.worktree / "notes.txt").exists()) + + def test_old_update_cleanup_preserves_persistent_assistant_worktree(self): + task = self._prepare() + IsolatedWriteSupervisor(self.root, "update").cleanup_stale() + self.assertTrue(task.worktree.is_dir()) + self.assertEqual(self._git("rev-parse", "HEAD", cwd=task.worktree), task.base_commit) + + def test_mismatched_task_record_is_rejected(self): + task = self._prepare() + path = self.manager._record_path(task.task_id) + record = json.loads(path.read_text()) + record["task_id"] = "another-task" + path.write_text(json.dumps(record)) + with self.assertRaisesRegex(ValueError, "invalid assistant task identity"): + self.manager.complete(task.task_id) + + @unittest.skipIf(os.name == "nt", "symlinks require Windows privileges") + def test_runtime_symlink_is_rejected_before_creating_files(self): + outside = self.root / "outside" + outside.mkdir() + (self.root / ".runtime").symlink_to(outside, target_is_directory=True) + with self.assertRaisesRegex(ValueError, "must not be a symlink"): + self._prepare() + self.assertEqual(list(outside.iterdir()), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_validation.py b/tests/test_validation.py new file mode 100644 index 0000000..05ee75f --- /dev/null +++ b/tests/test_validation.py @@ -0,0 +1,122 @@ +from __future__ import annotations + +import json +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from rightmemory.guidance import GUIDANCE_INBOX_PATH +from rightmemory.tools import MemoryTools +from rightmemory.validation import validate_memory_root + + +class CompleteMemoryValidationTests(unittest.TestCase): + def setUp(self) -> None: + self.tempdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tempdir.cleanup) + self.root = Path(self.tempdir.name) + (self.root / "MEMORY.md").write_text("# Memory\n", encoding="utf-8") + (self.root / "PURSUITS.md").write_text("# Pursuits\n", encoding="utf-8") + + def test_valid_root_is_model_free_readonly_and_json_serializable(self) -> None: + before = {path.name: path.read_bytes() for path in self.root.iterdir()} + + result = validate_memory_root(self.root) + + self.assertTrue(result.ok) + self.assertEqual(result.errors, ()) + self.assertIsNotNone(result.memory_report) + self.assertEqual(json.loads(json.dumps(result.to_dict()))["errors"], []) + self.assertEqual({path.name: path.read_bytes() for path in self.root.iterdir()}, before) + + def test_required_document_failure_prevents_graph_validation(self) -> None: + (self.root / "PURSUITS.md").unlink() + with patch.object(MemoryTools, "validate_memory", side_effect=AssertionError("invalid root read")): + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertEqual(len(result.document_errors), 1) + self.assertIsNone(result.memory_report) + + def test_legacy_reference_and_nonregular_corrections_are_rejected(self) -> None: + (self.root / "PURSUIT_RULES.md").write_text("test", encoding="utf-8") + (self.root / "corrections.md").mkdir() + + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertEqual(len(result.document_errors), 2) + + def test_symlinked_required_root_is_rejected(self) -> None: + memory = self.root / "MEMORY.md" + target = self.root / "other.md" + memory.rename(target) + try: + memory.symlink_to(target) + except OSError as exc: + self.skipTest(f"symlink creation failed: {exc}") + + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertEqual(len(result.document_errors), 1) + + def test_graph_failure_retains_original_report_and_structured_errors(self) -> None: + (self.root / "MEMORY.md").write_text("# Missing {F#missing}\n", encoding="utf-8") + + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertTrue(result.errors) + self.assertEqual(result.document_errors, ()) + self.assertEqual(result.guidance_errors, ()) + self.assertIsNotNone(result.memory_report) + + def test_guidance_is_validated_even_when_memory_is_valid(self) -> None: + (self.root / GUIDANCE_INBOX_PATH).write_text( + "# Pending Agent Guidance\n\n## GI-20260815-a1b2c3d4\n\nMissing provenance.\n", + encoding="utf-8", + ) + + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertTrue(result.guidance_errors) + self.assertEqual(result.errors, result.guidance_errors) + self.assertIsNotNone(result.memory_report) + + def test_guidance_regular_file_check_is_included_with_root_errors(self) -> None: + (self.root / "MEMORY.md").unlink() + (self.root / GUIDANCE_INBOX_PATH).mkdir() + + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertEqual(len(result.document_errors), 1) + self.assertEqual(len(result.guidance_errors), 1) + self.assertEqual(len(result.errors), 2) + + def test_correction_collection_limits_remain_enforced(self) -> None: + (self.root / "MEMORY_agent-corrections-writing.md").write_text( + "# Agent Corrections\n\n### 1. Entry\n\n" + "x" * 201 + "\n", + encoding="utf-8", + ) + + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertTrue(result.errors) + self.assertIsNotNone(result.memory_report) + + def test_unreadable_encoding_is_a_structured_failure(self) -> None: + (self.root / "MEMORY.md").write_bytes(b"\xff") + + result = validate_memory_root(self.root) + + self.assertFalse(result.ok) + self.assertEqual(len(result.errors), 1) + + +if __name__ == "__main__": + unittest.main() From 0aa4c18d625e1ebb6f5dd9c1fccde20f69b79eb4 Mon Sep 17 00:00:00 2001 From: RightL Date: Fri, 23 Oct 2026 20:58:50 +0800 Subject: [PATCH 2/3] Fix native assistant recovery and task handoff Preserve expired native Update tasks until explicit retry while retaining publication fencing. Supply concrete quoted task commands and explain completed worktree cleanup. Add focused expiry and shared-lease regressions. --- rightmemory/assistant.py | 5 +++++ rightmemory/assistant_prompt.py | 16 +++++++++++----- tests/test_assistant_service.py | 33 +++++++++++++++++++++++++++++++-- tests/test_assistant_sync.py | 24 ++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/rightmemory/assistant.py b/rightmemory/assistant.py index 138d62d..f20036d 100644 --- a/rightmemory/assistant.py +++ b/rightmemory/assistant.py @@ -123,6 +123,11 @@ def begin_update(self, retry_task: str | None = None) -> dict[str, Any]: from .assistant_write import AssistantStaleVersionError if not isinstance(exc, AssistantStaleVersionError): raise + if retry_task is None: + # Lease expiry is not permission to abandon native edits. + # Keep the task identity until an explicit retry, while + # publication still enforces the shared queue's fence. + return self._response(self._prepare(active, claimed=False)) sync_result = SyncManager(self.sync_config).pull() if sync_result.status not in {"disabled", "synced", "ahead", "pushed"}: diff --git a/rightmemory/assistant_prompt.py b/rightmemory/assistant_prompt.py index 65d44ad..a5ecb8f 100644 --- a/rightmemory/assistant_prompt.py +++ b/rightmemory/assistant_prompt.py @@ -1,5 +1,6 @@ from __future__ import annotations +import shlex from pathlib import Path from .prompt import _read_prompt_file, build_cli_agent_instructions @@ -27,6 +28,10 @@ def build_assistant_update_instructions( ) -> str: """Reuse Update judgment and native Git work, then return to program finalization.""" canonical = build_cli_agent_instructions(worktree, "update") + validate_command = shlex.join(["rightmemory", "validate", "--root", str(worktree)]) + control = ["rightmemory", "assistant", "--root", str(active_root), "update"] + complete_command = shlex.join([*control, "complete", "--task", task_id]) + status_command = shlex.join([*control, "status", "--task", task_id]) return f"""{canonical} Assistant execution: @@ -34,14 +39,15 @@ def build_assistant_update_instructions( - The active root for assistant control commands is {active_root}. Keep that binding when running commands from the candidate worktree. - Use native file and Git tools for the canonical Update role. Validate with - `rightmemory validate --root ` before committing the allowed changes. -- After the worktree is clean, call the task's supplied complete_command, or the - matching assistant MCP completion tool. The command is - `rightmemory assistant --root update complete --task `. + `{validate_command}` before committing the allowed changes. +- After the worktree is clean, run `{complete_command}`, or the matching + assistant MCP completion tool. Complete an unchanged task too so its batch can be recorded. - A worktree commit is candidate work. Only the program's completion receipt confirms application to the active root; report its application and sync outcome. -- If completion is interrupted or its reply is lost, inspect the same task's status + Successful completion removes the candidate worktree; run later checks from + the active root instead. +- If completion is interrupted or its reply is lost, run `{status_command}` and retry its completion as directed. Preserve the worktree and task identity. A prepared result awaiting publication needs the same completion retry, not another semantic edit of its candidate. diff --git a/tests/test_assistant_service.py b/tests/test_assistant_service.py index 947e7b4..658fa3f 100644 --- a/tests/test_assistant_service.py +++ b/tests/test_assistant_service.py @@ -126,11 +126,40 @@ def test_stale_edit_retries_same_batch_with_new_attempt_and_fences_old_task(self self.assertIn("Concurrent evidence", memory) self.assertIn("Orion", memory) - def test_expired_unfinished_claim_can_be_replaced_and_old_no_change_is_fenced(self): + def test_expired_ordinary_begin_resumes_native_edits_without_replacing_task(self): + task = self._begin() + self._edit(task) + candidate = self._git("rev-parse", "HEAD", cwd=Path(task["worktree"])) + later = datetime(2100, 1, 1, tzinfo=UTC) + with patch("rightmemory.assistant._now", return_value=later): + resumed = AssistantService(self.root).begin_update() + self.assertFalse(resumed["claimed"]) + self.assertEqual(resumed["task_id"], task["task_id"]) + self.assertEqual(resumed["worktree"], task["worktree"]) + self.assertEqual(self._git("rev-parse", "HEAD", cwd=Path(task["worktree"])), candidate) + self.assertEqual(self._git("rev-parse", "HEAD"), self.initial_head) + completed = self.service.complete_update(task["task_id"]) + self.assertEqual(completed["status"], "completed") + self.assertIn("Orion", (self.root / "MEMORY.md").read_text()) + + def test_expired_ordinary_begin_recovers_prepared_result_without_replacing_task(self): + task = self._begin() + self._edit(task) + with patch.object(self.service.writer.supervisor, "publish_external", side_effect=OSError("interrupted")): + with self.assertRaises(OSError): + self.service.complete_update(task["task_id"]) + later = datetime(2100, 1, 1, tzinfo=UTC) + with patch("rightmemory.assistant._now", return_value=later): + recovered = AssistantService(self.root).begin_update() + self.assertEqual(recovered["status"], "completed") + self.assertEqual(recovered["task_id"], task["task_id"]) + self.assertEqual(self._git("rev-list", "--count", f"{self.initial_head}..HEAD"), "1") + + def test_expired_unfinished_claim_can_be_explicitly_replaced_and_old_no_change_is_fenced(self): task = self._begin() later = datetime(2100, 1, 1, tzinfo=UTC) with patch("rightmemory.assistant._now", return_value=later): - replacement = self.service.begin_update() + replacement = self.service.begin_update(retry_task=task["task_id"]) self.assertNotEqual(task["task_id"], replacement["task_id"]) with self.assertRaises(AssistantClaimLost): self.service.complete_update(task["task_id"]) diff --git a/tests/test_assistant_sync.py b/tests/test_assistant_sync.py index 9fdfce9..a1e80ef 100644 --- a/tests/test_assistant_sync.py +++ b/tests/test_assistant_sync.py @@ -210,6 +210,30 @@ def test_remote_lease_takeover_fences_old_native_task(self): self.assertIn("current owner result", memory) self.assertNotIn("obsolete attempt", memory) + def test_expired_ordinary_begin_keeps_native_task_but_does_not_bypass_foreign_lease(self): + _submission, old = self._begin() + self._commit_memory(old, key="old-owner", value="retained native attempt") + expires = datetime.fromisoformat(old["expires_at"]) + timedelta(seconds=1) + with ( + patch("rightmemory.update_queue_git._utc_now", return_value=expires), + patch("rightmemory.assistant._now", return_value=expires), + ): + resumed = AssistantService(self.first).begin_update() + self.assertFalse(resumed["claimed"]) + self.assertEqual(resumed["task_id"], old["task_id"]) + self.assertEqual(resumed["worktree"], old["worktree"]) + replacement = self.other.begin_update() + self.assertTrue(replacement["claimed"]) + remote_head = self._remote_head() + with self.assertRaises(UpdateQueueLeaseLost): + self.service.complete_update(old["task_id"]) + resumed_after_takeover = AssistantService(self.first).begin_update() + self.assertFalse(resumed_after_takeover["claimed"]) + self.assertEqual(resumed_after_takeover["task_id"], old["task_id"]) + self.assertTrue(Path(old["worktree"]).is_dir()) + self.assertEqual(self._remote_head(), remote_head) + self.assertNotIn("retained native attempt", self._git(self.remote, "show", "main:MEMORY.md")) + def test_ordinary_pull_conflict_stops_update_without_internal_model_repair(self): for root, value in ((self.first, "local fact"), (self.second, "remote fact")): (root / "MEMORY.md").write_text(f"# Domain\n\n- `one` {value} → []\n", encoding="utf-8") From d5191d3730f19ca470c2791f36714acf104dd0b8 Mon Sep 17 00:00:00 2001 From: RightL Date: Mon, 26 Oct 2026 19:16:41 +0800 Subject: [PATCH 3/3] Prepare native assistant execution for release Add explicit assistant installer mode using existing root and runtime provisioning. Bind the native skill to the selected root, avoid internal worker startup, and refuse pending cross-executor mode switches. Replace stale experimental verification notes with supported setup and operational guidance. --- README.md | 2 +- docs/ASSISTANT_EXECUTION.md | 53 ++----------- install.ps1 | 2 +- install.sh | 2 +- rightmemory/install_core.py | 66 ++++++++++++++++- skills/rightmemory-assistant/SKILL.md | 2 +- tests/test_install_core.py | 102 +++++++++++++++++++++++++- 7 files changed, 174 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index c116144..859c7fb 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ RightMemory is aimed at developers who spend serious time with coding agents and ## Quick Start -To use your current assistant's native agents without a RightMemory model backend, follow [Assistant execution](docs/ASSISTANT_EXECUTION.md). It uses `rightmemory assistant`, a shared skill, and native file tools. The installer paths below configure the existing internal runtime. +To use your current assistant's native agents without a RightMemory model backend, follow [Assistant execution](docs/ASSISTANT_EXECUTION.md). It uses `rightmemory assistant`, a shared skill, and native file tools. Use the installer’s `--mode assistant` with explicit memory-root and skill-directory paths. The default installer paths below configure the existing internal runtime. Install prerequisites: diff --git a/docs/ASSISTANT_EXECUTION.md b/docs/ASSISTANT_EXECUTION.md index 7af5183..d722694 100644 --- a/docs/ASSISTANT_EXECUTION.md +++ b/docs/ASSISTANT_EXECUTION.md @@ -6,13 +6,15 @@ This path uses one shared [assistant skill](../skills/rightmemory-assistant/SKIL ## Setup -Install the package in a Python 3.11+ environment on the computer that will hold the memory root: +From the repository checkout, run the shared installer with explicit memory and skill directories: ```bash -python -m pip install . +./install.sh --mode assistant /absolute/memory-root /absolute/assistant-skills ``` -Use a complete RightMemory Git root with a configured Git author. A disposable root is appropriate for initial verification. Copy `skills/rightmemory-assistant/` from the same checkout into the assistant's supported skill location and enable it in the conversation. The existing `install.sh` and `install.ps1` modes do not select assistant execution or install this skill automatically. +On Windows, use `./install.ps1 --mode assistant C:\path\memory C:\path\assistant-skills` in PowerShell. The installer uses Git and uv, provisions Python 3.11+ and the runtime, initializes a new memory root, and installs `rightmemory-assistant` with that root bound. Existing complete roots are preserved; incomplete roots are refused before changes. Enable the installed skill using your assistant's supported skill mechanism. The skills directory is an explicit host choice, not automatic dot plugin registration. + +Use `rightmemory validate --root /absolute/memory-root` to check the root. Replace the new root's example entries with your own state through the normal memory workflow. Re-run the same installer command after updating the checkout. Stop agents and separately configured watchers that can submit evidence before switching execution modes. Finish or cancel pending updates with their original executor first; the installer refuses a switch while it finds their pending work. Use assistant execution rather than an internal Update worker for this root. If an old orchestrator skill was customized and therefore preserved, disable it before enabling the native assistant skill. The default standalone and CLI-agent installer modes remain unchanged. Assistant execution does not require RightMemory model API credentials, Codex, or Claude Code. The assistant uses its own configured agent. Keep the active root explicit: @@ -81,7 +83,7 @@ rightmemory assistant --root /absolute/memory-root sync pull rightmemory assistant --root /absolute/memory-root sync push ``` -An incoming conflict or invalid merged state returns sync status and available file/operation details, and blocks a new Update. This assistant interface does not yet provide a native-agent sync-repair workspace protocol. The existing separate repair paths remain available; resolve the conflict before retrying ordinary sync. +An incoming conflict or invalid merged state returns sync status and available file/operation details, and blocks a new Update. Resolve the reported conflict using the existing sync repair workflow, then retry ordinary sync before preparing another Update. Sync conflicts are not delegated to a native repair agent by this interface. The optional MCP transport wraps the same assistant core: @@ -91,45 +93,4 @@ rightmemory assistant --root /absolute/memory-root mcp --transport stdio Load the shared assistant skill with this MCP transport; it explains how the assistant's own agents use the deterministic tools. The older `rightmemory mcp` command continues to use its configured internal role backend. See [MCP](MCP.md). -The assistant path is not a replacement for all internal-runtime behavior. It does not promise the old model-session history, incremental retrieval cache, delivery suppression, transcript watchers, Dreamer, or scheduled maintenance. Use the existing paths for those mechanisms unless separately supported. The public assistant queue and completion receipts cover native batch execution rather than pretending that the old `update submit` worker queue dispatches native subagents. - -## Dot Verification - -Run the following in an explicitly authorized disposable memory root. Do not connect real memory, create credentials, or enable a persistent connection merely to run this checklist. In that root's `rightmemory.toml`, use a small test batch so one submitted candidate becomes ready immediately: - -```toml -[update.async] -trigger_candidates = 1 -target_batch_candidates = 1 -max_wait_seconds = 1 -``` - -The normal defaults are 15 candidates to trigger, 30 as the preferred batch size, and a 24-hour fallback after the quiet period. The small values above are for this disposable exercise. - -1. **Check the actual execution environment.** Let a native Retrieve subagent read a fictional marker stored in a valid memory entry using its own file tools. During Update, let its native subagent modify an allowed entry in the assigned worktree, then have RightMemory inspect that candidate. This establishes shared filesystem access; MCP discovery alone does not. -2. **Exercise the shared skill.** Submit a fictional durable fact. Before reconciliation, have a fresh Retrieve subagent find it as pending evidence. Trigger a normal tool call that returns a ready-batch notice to the subagent and verify that the main agent receives it and delegates Update. -3. **Finish a real Update.** Let the native Update subagent read the candidate, decide and make the edit in the assigned worktree, run standalone validation, commit, and complete the task. Inspect the program's outcome and the active root's Git change. -4. **Read it independently.** Start a fresh Retrieve subagent with a question that requires the new fact, without giving it the answer or the Update transcript. Check the returned source against the committed memory. -5. **Exercise failure cases.** Run the cases below against disposable state and inspect both operation receipts and Git state. - -| Case | Required result | -| --- | --- | -| Same submission retried | The original submission is returned; evidence is not duplicated. Reusing its id for different content fails. | -| Active root changed after `begin` | Completion reports a conflict; it does not overwrite the newer state. | -| Invalid path or non-regular file | Candidate admission rejects unsafe Git paths or file types; task and session ids cannot escape their state directory. No outside path is changed through the program. | -| Invalid candidate graph | Independent validation and completion reject it; active memory stays unchanged. | -| Interrupted before application | The task remains inspectable and recoverable; no partial active update appears. | -| Applied, reply lost | Status or repeated completion reports the same outcome; no second memory commit is created. | -| Shared-queue publication failed | The durable prepared result remains; repeating completion retries publication without repeating semantic Update. | - -Record the assistant/version, RightMemory commit, root and worktree locations, native agent identities, task id, validation result, applied commit, and sync outcome. A check passes only when its observed output supports that result. - -## Assistant Verification Status - -| Assistant | What the existing evidence establishes | Still to verify for this path | -| --- | --- | --- | -| dot | An earlier isolated stdio handshake, three-tool discovery/error handling, and a limited synthetic write/read exercise. | The native-agent batch flow and failure/recovery cases above; native plugin and filesystem access in the intended deployment. | -| DeepSeek Harness | No completed test of this path is recorded. | This shared skill and tool flow in DSH, using its native provider and filesystem. | -| WorkBuddy | No completed test of this path in WorkBuddy itself is recorded. | The full flow in WorkBuddy itself, including native delegation and worktree access. CodeBuddy results do not establish this. | - -These are verification boundaries, not claims that a product has been adapted. Private dot MCP access may use [OpenAI Secure MCP Tunnel](https://developers.openai.com/api/docs/guides/secure-mcp-tunnels), including its direct stdio bridge; tunnel authentication and a persistent client require separate setup and authorization. +Assistant execution covers native retrieval and batch updates. Internal model-session history, incremental retrieval caching, transcript watchers, Dreamer, and scheduled maintenance belong to the separate internal runtime. Use `assistant submit` and its completion receipts for native batches, not the internal `update submit` worker commands. diff --git a/install.ps1 b/install.ps1 index 88988c4..9033f77 100644 --- a/install.ps1 +++ b/install.ps1 @@ -6,7 +6,7 @@ if (Get-Variable -Name PSNativeCommandUseErrorActionPreference -ErrorAction Sile } if ($args -contains "-h" -or $args -contains "--help") { - Write-Host "Usage: .\install.ps1 [--mode cli-agent|standalone] [ ]" + Write-Host "Usage: .\install.ps1 [--mode assistant|cli-agent|standalone] [ ]" exit 0 } diff --git a/install.sh b/install.sh index ba74505..9cd3e6c 100755 --- a/install.sh +++ b/install.sh @@ -5,7 +5,7 @@ set -u for argument in "$@"; do if [ "$argument" = "-h" ] || [ "$argument" = "--help" ]; then - echo "Usage: ./install.sh [--mode cli-agent|standalone] [ ]" + echo "Usage: ./install.sh [--mode assistant|cli-agent|standalone] [ ]" exit 0 fi done diff --git a/rightmemory/install_core.py b/rightmemory/install_core.py index 1f398c5..b316fbb 100644 --- a/rightmemory/install_core.py +++ b/rightmemory/install_core.py @@ -119,11 +119,13 @@ def run(self) -> None: self._require_valid_update_records() self._require_no_live_legacy_async_updates() self._require_no_legacy_review_deliveries() + self._require_compatible_update_executor() target = self._inspect_target() self._print_layout() self._require_complete_existing_target(target) with self._async_update_install_boundary() as worker_was_running: + self._require_compatible_update_executor() if target.kind == "new": self._bootstrap_state() else: @@ -252,6 +254,32 @@ def _require_no_legacy_review_deliveries(self) -> None: "to finish these deliveries, then rerun the installer" ) + def _require_compatible_update_executor(self) -> None: + expected = "assistant" if self.mode == "assistant" else "worker" + async_root = self.memory_root / ".runtime" / "async" / "update" + states = { + path.stem: _state_from_json(json.loads(path.read_text(encoding="utf-8"))) + for path in sorted(async_root.glob("*.json")) + } + incompatible = { + session for session, state in states.items() + if state.executor != expected and (state.pending or state.current_batch) + } + # An interrupted submission can have durable outbox evidence before its + # session's pending list is written. Missing sessions use the worker default. + for candidate in UpdateQueueStore(self.memory_root).outbox_candidates(cleanup_temporary=False): + state = states.get(candidate.session_id) + executor = state.executor if state is not None else "worker" + if executor != expected: + incompatible.add(candidate.session_id) + if incompatible: + raise InstallError( + "pending updates belong to another execution mode: " + + ", ".join(sorted(incompatible)) + + "; installation made no changes. Finish or cancel those updates " + "with their current executor before switching modes." + ) + @contextmanager def _async_update_install_boundary(self): try: @@ -267,6 +295,8 @@ def _async_update_install_boundary(self): ) from exc def _restart_async_update_worker(self, worker_was_running: bool) -> None: + if self.mode == "assistant": + return if not worker_was_running and not self._async_update_work_remains(): return if self.runtime_python is None: @@ -623,6 +653,23 @@ def _run_semantic_upgrades(self, target: InstallTarget) -> None: def _install_skills(self) -> None: for target in self.skills_targets: target.mkdir(parents=True, exist_ok=True) + if self.mode == "assistant": + self._remove_managed_skill( + self.repo_root / "skills" / "rightmemory-auto-orchestrator-cli" / "SKILL.md", + "rightmemory-auto-orchestrator", + target, + ) + self._install_skill( + self.repo_root / "skills" / "rightmemory-assistant" / "SKILL.md", + "rightmemory-assistant", + target, + ) + continue + self._remove_managed_skill( + self.repo_root / "skills" / "rightmemory-assistant" / "SKILL.md", + "rightmemory-assistant", + target, + ) self._remove_old_loose_reference( target, "rightmemory-schema.md", @@ -794,8 +841,9 @@ def _write_install_stamp(self) -> None: install_stamp = runtime_dir / "install.stamp" _write_utf8(install_stamp, stamp) print(f" [refresh] {install_stamp}") - print(" running watch processes refresh after their current cycle or sleep check") - print(" run rightmemory watch start or restart to start newly added watch targets") + if self.mode != "assistant": + print(" running watch processes refresh after their current cycle or sleep check") + print(" run rightmemory watch start or restart to start newly added watch targets") def _print_next_steps(self) -> None: separator = "\\" if self.is_windows else "/" @@ -805,6 +853,14 @@ def _print_next_steps(self) -> None: f" 1. Open {self.memory_root}{separator}MEMORY.md and " f"{self.memory_root}{separator}PURSUITS.md and replace the examples with your own state." ) + if self.mode == "assistant": + print(" 2. Enable the installed rightmemory-assistant skill in your assistant.") + print(" The main agent and native subagents need access to this root and its worktrees.") + print(" No RightMemory model API or CLI-agent backend configuration is needed.") + print(f" 3. Check the root: rightmemory validate --root {self.memory_root}") + print(" Use rightmemory assistant commands; do not run an internal Update worker on this root.") + print("Re-run this installer with --mode assistant and the same paths to update the runtime and skill.") + return if self.mode == "cli-agent": print( " 2. Write [agent_cli], [retrieve.agent_cli], and a default writer [update.agent_cli] " @@ -925,17 +981,19 @@ def _resolved_path(path: str | Path) -> Path: def _parse_args(argv: list[str] | None) -> argparse.Namespace: parser = argparse.ArgumentParser( prog="install.ps1" if os.name == "nt" else "install.sh", - usage="%(prog)s [--mode cli-agent|standalone] [ ]", + usage="%(prog)s [--mode assistant|cli-agent|standalone] [ ]", ) parser.add_argument("--mode", default="standalone") parser.add_argument("paths", nargs="*") args = parser.parse_args(argv) if args.mode == "subagent": parser.exit(1, "Unsupported --mode: subagent\nUse --mode cli-agent for command-backed agent skill installs.\n") - if args.mode not in {"cli-agent", "standalone"}: + if args.mode not in {"assistant", "cli-agent", "standalone"}: parser.error(f"Invalid --mode: {args.mode}") if len(args.paths) not in {0, 2}: parser.error("provide either no paths or both and ") + if args.mode == "assistant" and not args.paths: + parser.error("assistant mode requires explicit and paths") return args diff --git a/skills/rightmemory-assistant/SKILL.md b/skills/rightmemory-assistant/SKILL.md index e05e454..2a90858 100644 --- a/skills/rightmemory-assistant/SKILL.md +++ b/skills/rightmemory-assistant/SKILL.md @@ -5,7 +5,7 @@ description: "Use when the user chooses RightMemory assistant execution: retriev # Use RightMemory with Native Agents -Use the configured RightMemory root and one stable session id throughout the conversation. Use its absolute path as `` in every assistant command. The main agent owns submissions and Update delegation. Retrieve and Update run as native subagents with access to the supplied directories; RightMemory tools manage the queue, isolation, validation, and publication. +The installed memory root is `{{MEMORY_ROOT}}`. Use that root and one stable session id throughout the conversation. Use its absolute path as `` in every assistant command. The main agent owns submissions and Update delegation. Retrieve and Update run as native subagents with access to the supplied directories; RightMemory tools manage the queue, isolation, validation, and publication. Use the `rightmemory assistant` commands below, or their matching assistant MCP tools. Keep this skill active with either transport. diff --git a/tests/test_install_core.py b/tests/test_install_core.py index a582673..bf5d4b5 100644 --- a/tests/test_install_core.py +++ b/tests/test_install_core.py @@ -7,6 +7,7 @@ from pathlib import Path from unittest.mock import patch +from rightmemory.async_update import AsyncUpdateJob, AsyncUpdateState from rightmemory import install_core from rightmemory.install_core import InstallError, InstallTarget, Installer, _posix_data_home from rightmemory.update_queue import UpdateCandidate, UpdateQueueStore @@ -76,7 +77,7 @@ def find_spec(self, fullname, path=None, target=None): def test_runtime_install_adds_codex_extra_only_for_cli_agent_mode(self): with tempfile.TemporaryDirectory() as tempdir: root = Path(tempdir) - for mode, suffix in (("standalone", ""), ("cli-agent", "[codex-sdk]")): + for mode, suffix in (("standalone", ""), ("assistant", ""), ("cli-agent", "[codex-sdk]")): with self.subTest(mode=mode): installer = Installer(REPO_ROOT, mode, root / "memory", [root / "skills"]) installer.runtime_home = root / f"runtime-{mode}" @@ -109,6 +110,105 @@ def test_runtime_install_adds_codex_extra_only_for_cli_agent_mode(self): ] ) + def test_assistant_mode_requires_explicit_paths(self): + with patch("sys.stderr"), self.assertRaises(SystemExit): + install_core._parse_args(["--mode", "assistant"]) + args = install_core._parse_args(["--mode", "assistant", "/memory", "/skills"]) + self.assertEqual(args.mode, "assistant") + self.assertEqual(args.paths, ["/memory", "/skills"]) + + def test_assistant_mode_does_not_start_internal_worker(self): + with tempfile.TemporaryDirectory() as tempdir: + root = Path(tempdir) + installer = Installer(REPO_ROOT, "assistant", root / "memory", [root / "skills"]) + with ( + patch.object(installer, "_async_update_work_remains") as remains, + patch.object(install_core.AsyncUpdateStore, "wake_worker") as wake, + ): + installer._restart_async_update_worker(True) + installer._restart_async_update_worker(False) + remains.assert_not_called() + wake.assert_not_called() + + def test_assistant_mode_selects_native_skill_instead_of_internal_orchestrator(self): + with tempfile.TemporaryDirectory() as tempdir: + root = Path(tempdir) + target = root / "skills" + installer = Installer(REPO_ROOT, "assistant", root / "memory", [target]) + with ( + patch.object(installer, "_install_skill") as install, + patch.object(installer, "_remove_managed_skill") as remove, + patch("builtins.print"), + ): + installer._install_skills() + install.assert_called_once_with( + REPO_ROOT / "skills" / "rightmemory-assistant" / "SKILL.md", + "rightmemory-assistant", target, + ) + remove.assert_called_once_with( + REPO_ROOT / "skills" / "rightmemory-auto-orchestrator-cli" / "SKILL.md", + "rightmemory-auto-orchestrator", target, + ) + + def test_internal_mode_removes_managed_native_skill(self): + with tempfile.TemporaryDirectory() as tempdir: + root = Path(tempdir) + target = root / "skills" + installer = Installer(REPO_ROOT, "standalone", root / "memory", [target]) + with ( + patch.object(installer, "_install_skill"), + patch.object(installer, "_remove_managed_skill") as remove, + patch.object(installer, "_remove_old_skill"), + patch.object(installer, "_remove_old_loose_reference"), + patch("builtins.print"), + ): + installer._install_skills() + remove.assert_any_call( + REPO_ROOT / "skills" / "rightmemory-assistant" / "SKILL.md", + "rightmemory-assistant", target, + ) + + def test_mode_switch_refuses_other_executors_pending_work_without_changes(self): + for old, new in (("worker", "assistant"), ("assistant", "standalone")): + with self.subTest(old=old, new=new), tempfile.TemporaryDirectory() as tempdir: + root = Path(tempdir) + memory = root / "memory" + store = install_core.AsyncUpdateStore(memory, "update", executor=old) + job = AsyncUpdateJob(1, "a" * 32, "fixture evidence", "2026-01-01T00:00:00+00:00") + state = AsyncUpdateState("running", "fixture", "update", phase="waiting", pending=[job], next_id=2, executor=old) + store._write("fixture", state) + before = _snapshot(memory) + installer = Installer(REPO_ROOT, new, memory, [root / "skills"]) + with self.assertRaisesRegex(InstallError, "another execution mode"): + installer.run() + self.assertEqual(_snapshot(memory), before) + self.assertFalse((root / "skills").exists()) + + def test_assistant_mode_refuses_orphan_worker_outbox_without_cleanup(self): + with tempfile.TemporaryDirectory() as tempdir: + memory = Path(tempdir) / "memory" + queue = UpdateQueueStore(memory) + queue.write_outbox(UpdateCandidate( + uid="a" * 32, submitted_at="2026-01-01T00:00:00+00:00", + display_id=1, session_id="interrupted-worker", message="fixture evidence", + )) + temporary = queue.outbox_root / (".999999999." + "b" * 32 + ".tmp") + temporary.write_text("preserve on refusal", encoding="utf-8") + before = _snapshot(memory) + with self.assertRaisesRegex(InstallError, "another execution mode"): + Installer(REPO_ROOT, "assistant", memory, [])._require_compatible_update_executor() + self.assertEqual(_snapshot(memory), before) + + def test_same_executor_reinstall_preserves_pending_work(self): + with tempfile.TemporaryDirectory() as tempdir: + memory = Path(tempdir) / "memory" + store = install_core.AsyncUpdateStore(memory, "update", executor="assistant") + job = AsyncUpdateJob(1, "a" * 32, "fixture evidence", "2026-01-01T00:00:00+00:00") + store._write("fixture", AsyncUpdateState("running", "fixture", "update", phase="waiting", pending=[job], next_id=2, executor="assistant")) + before = _snapshot(memory) + Installer(REPO_ROOT, "assistant", memory, [])._require_compatible_update_executor() + self.assertEqual(_snapshot(memory), before) + def test_empty_xdg_data_home_uses_standard_default(self): home = Path("test-home")