Skip to content

feat: refine agent memory management operations - #31

Open
faj-design5260 wants to merge 2 commits into
mainfrom
feat/manage-operation-boundaries
Open

faj-design5260 wants to merge 2 commits into
mainfrom
feat/manage-operation-boundaries

Conversation

@faj-design5260

@faj-design5260 faj-design5260 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Agent management already exposes record/correct and automatic relation maintenance. This PR tightens the existing correction operation boundary: explicit relationships must point to distinct active memories, CLI and MCP can replace or clear the full list, and concurrent corrections cannot overwrite each other's edits.

Why

These are correctness fixes to existing operations. A correction that reads before acquiring the writer lock can silently lose another writer's update. A relationship correction that accepts a missing or invalid target can leave canonical memory in an inconsistent state. Differing CLI and MCP contracts make that state dependent on the entry point.

Implementation

  • Hold the store writer lock across correction read, validation, persistence, and projection. Store.correct and Store.write share one locked persistence path, with no nested lock.
  • Require an active correction source and active successor. Explicit relationship replacement validates every submitted target, including one previously stored but now invalid; it rejects missing, inactive, self, and duplicate targets before mutation. Omitted links preserve historical relationships. An empty list clears them.
  • Route mem correct and MCP memory_correct to the same Store operation. CLI --link repeats to specify the complete replacement list, while --clear-links matches MCP links: []. MCP rejects malformed arrays before dispatch.
  • Preserve target memories and evidence when references are removed. Canonical files remain the source of truth and successful corrections update the projection; rebuild reproduces the resulting state.
  • Document the operation contract and existing management limits in the design note and audit.

Validation

  • Full suite: 426 passed (uv run --no-sync --offline pytest -q, using the worktree source paths and existing Python 3.12 environment).
  • Coverage: 426 passed, 91.24% (--cov=agent_memory --cov-report=term-missing --cov-fail-under=85).
  • Targeted storage, indexer, management, concurrency, CLI and MCP tests: 59 passed.
  • Ruff: PASS. Mypy: PASS, 66 source files. git diff --check: PASS.
  • Cross-process correction regression: PASS; independent Store instances retain both abstract and body updates, with canonical and index state checked.
  • Real Store mechanics smoke: PASS; A→B link replacement, MCP clear, inactive target rejection, canonical preservation, and rebuild consistency.
  • Rebased against latest origin/main (34d12a2f): yes; rebase reported up to date.

Scope and non-goals

This PR only refines existing Manage/correction boundaries. It does not add RBAC, approval or confirmation workflow, standalone unlink, archive management, retrieval improvements, answer-level benchmark optimization, general rollback, new lifecycle behavior, or feedback redesign. Existing automatic linking and proposal controls remain.

The change was split from #29 for independent review and requires no API from #29.

Follow-ups

  • Store.feedback returns an adjusted weight without persisting it; tracked in docs/TODO.md, outside this correction PR.
  • Manage can prepare a stale record before calling Store.write; that broader case remains for a separate change. Related to Store.correct() has a TOCTOU race between find and write #16; this PR fixes concurrent correct calls but does not claim to close the issue's Manage scenario.
  • Preimage retention, multi-file recovery, and operator authorization remain policy/design follow-ups in the operation audit.

Checklist

  • Implementation
  • Unit tests
  • Cross-process concurrency regression
  • CLI/MCP consistency
  • Design documentation
  • Full test suite and coverage
  • Lint and type checks
  • Mechanics smoke
  • Rebased on latest main

@faj-design5260

Copy link
Copy Markdown
Collaborator Author

Ready-to-merge validation completed on b49f1e6.

  • Full test suite: 426 passed.
  • Coverage: 91.24% (426 passed; 85% threshold).
  • Ruff: PASS.
  • Mypy: PASS (66 source files).
  • Cross-process concurrency regression: PASS; both corrections and projection retained.
  • CLI/MCP boundary tests: PASS (included in 59 targeted tests).
  • Mechanics smoke: PASS (replace, clear, reject invalid target, rebuild).
  • Rebased on latest main: yes (34d12a2f; already up to date).
  • Latest PR CI: check PASS on this HEAD.

No merge performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant