feat(memory): entry-policy convention, Agents template, memory template updates, and memory tool description improvements - #307
Conversation
Memory files gain a machine-readable entry-policy frontmatter property: append-only (the default; absent means this) or living (a current-state file whose expired entries are pruned rather than left as history). - vault_list_memory_files surfaces entry_policy per file (unrecognized values resolve to append-only — the safe reading) - vault_update_memory / vault_delete_memory descriptions state the default and the living exception; vault_delete_memory now names the case where deletion is intended maintenance - memory-review prompt labels each file's policy in its structural overview and gains a living-files-only reflection step for expired entries; append-only files keep the never-prune contract - New Agents template (6th memory file): directives for how agents communicate, work, and verify — split from facts about the user; templates README documents the who-is-the-subject routing test - Routines template becomes the living example, restructured to Active commitments / Upcoming / Daily-weekly rhythm / Recent past - All templates (in-code specs + templates/memory/) declare their policy, enumerate their sections in the scope callout, and cross-reference Agents; shrink-guard floor recalibrated to sit above the new largest empty template (Routines 1228 B) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The SHRINK_FLOOR_BYTES comment now depends on specific template sizes (Routines, Agents, Me, Opinions, Principles); consider deriving this threshold from actual template files at runtime or a shared constant so future template changes don’t silently invalidate the guard.
- entryPolicyFromFrontmatter currently treats anything except the exact string "living" as append-only; if you ever expand policies, it may be safer to validate against an explicit union (or enum) and surface invalid values to callers rather than silently coercing them to append-only.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The SHRINK_FLOOR_BYTES comment now depends on specific template sizes (Routines, Agents, Me, Opinions, Principles); consider deriving this threshold from actual template files at runtime or a shared constant so future template changes don’t silently invalidate the guard.
- entryPolicyFromFrontmatter currently treats anything except the exact string "living" as append-only; if you ever expand policies, it may be safer to validate against an explicit union (or enum) and surface invalid values to callers rather than silently coercing them to append-only.
## Individual Comments
### Comment 1
<location path="src/vault-mcp/mcp-core/__tests__/tool-definitions.test.ts" line_range="172-180" />
<code_context>
+ )
+ })
+
+ it("memory tool descriptions document the entry-policy contract", () => {
+ // Append-only is the default; the living opt-in must be discoverable from
+ // the tools that write, delete, and list memory — not only from templates.
+ const [, updateConfig] = requireCall(TOOL_NAMES.VAULT_UPDATE_MEMORY)
+ expect(updateConfig.description).toContain("entry-policy: living")
+ const [, deleteConfig] = requireCall(TOOL_NAMES.VAULT_DELETE_MEMORY)
+ expect(deleteConfig.description).toContain("entry-policy: living")
+ const [, listConfig] = requireCall(TOOL_NAMES.VAULT_LIST_MEMORY_FILES)
+ expect(listConfig.description).toContain(
+ 'entry_policy is "append-only" (the default',
)
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the memory tools contract test by also asserting the append-only default wording for update/delete tools.
The test now checks that all three memory tools mention `entry-policy: living` and that `vault_list_memory_files` documents the default append-only behavior. To fully capture the contract, also assert that `vault_update_memory` and `vault_delete_memory` include the explicit "append-only by default" wording (e.g., `expect(updateConfig.description).toContain("append-only by default")` and similarly for `deleteConfig`). This will better guard against regressions where the append-only default is omitted from their descriptions.
</issue_to_address>
### Comment 2
<location path="templates/memory/Routines.md" line_range="20" />
<code_context>
+> **Contains:** Active commitments, upcoming plans, recurring rhythms, and recent-past events kept for context — the time-sensitive logistics of the user's current life. A **current-state snapshot**, not a history ledger.
+> **Does NOT contain:** One-off events or reference material, identity facts (→ Me), principles (→ Principles), directives for AI agents (→ Agents).
+> **Section structure:** Active commitments, Upcoming, Daily/weekly rhythm, Recent past — each suffixed "(newest first)".
+> **Convention:** append newest first; ISO dates only. Entry policy: **living** (declared in frontmatter) — a deliberate exception to the memory layer's append-only default. When an Upcoming or Active-commitments entry expires, delete it and, if the outcome is worth keeping, append it to Recent past. Recent past entries are dated history and are not pruned.
-## Daily (newest first)
</code_context>
<issue_to_address>
**suggestion (typo):** Fix the hyphenation in “Active-commitments entry” to match the section name.
The hyphen makes the phrase look like a typo and inconsistent with the `Active commitments (newest first)` header; removing it will align the wording and improve readability.
```suggestion
> **Convention:** append newest first; ISO dates only. Entry policy: **living** (declared in frontmatter) — a deliberate exception to the memory layer's append-only default. When an Upcoming or Active commitments entry expires, delete it and, if the outcome is worth keeping, append it to Recent past. Recent past entries are dated history and are not pruned.
```
</issue_to_address>
### Comment 3
<location path="templates/memory/README.md" line_range="56-59" />
<code_context>
+
+Mixing the two is the most common drift in a lived-in memory layer: directives
+accumulate inside Principles and Me because they _feel_ like values. Route them
+to Agents.md from the start — it doubles as the highest-value always-read for
+any agent session.
+
+## Entry policy
</code_context>
<issue_to_address>
**nitpick (typo):** Clarify the noun phrase “highest-value always-read” for better grammar.
Consider clarifying what is meant by “always-read” by adding a specific noun, e.g., “highest-value always-read file” or “always-read document,” so the phrase is grammatically clear to readers.
```suggestion
Mixing the two is the most common drift in a lived-in memory layer: directives
accumulate inside Principles and Me because they _feel_ like values. Route them
to Agents.md from the start — it doubles as the highest-value always-read file
for any agent session.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
PR Code Suggestions ✨Explore these optional code suggestions:
|
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
|
@CodeRabbit review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughAdds explicit ChangesMemory entry policy and templates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vault-mcp/mcp-core/prompts/memory-review-prompt.ts`:
- Around line 206-208: Resolve the contradiction between step 6 and the
subsequent change instruction in the memory-review prompt. Update the
instruction near “Propose every change” so expired entries from living files are
removed via vault_delete_memory, while vault_update_memory remains required for
other edits and all operations still require confirmation before writing.
In `@src/vault-mcp/vault-operations/memory-store.ts`:
- Around line 21-26: Lower SHRINK_FLOOR_BYTES from 1300 to approximately 1250 so
a Routines file with one dated entry exceeds the guard threshold while empty
templates remain below it. Update the adjacent explanatory comment to reflect
the revised threshold and preserved empty-template behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5629a339-f988-4842-8c26-fb488142446b
📒 Files selected for processing (17)
.devin/wiki.jsonAGENTS.mdARCHITECTURE.mddeploy/local/README.mddeploy/remote/README.mdsrc/vault-mcp/mcp-core/__tests__/memory-review-prompt.test.tssrc/vault-mcp/mcp-core/__tests__/tool-definitions.test.tssrc/vault-mcp/mcp-core/prompts/memory-review-prompt.tssrc/vault-mcp/mcp-core/tools/memory-tools.tssrc/vault-mcp/vault-operations/__tests__/memory-store.test.tssrc/vault-mcp/vault-operations/memory-store.tstemplates/memory/Agents.mdtemplates/memory/Me.mdtemplates/memory/Opinions.mdtemplates/memory/Principles.mdtemplates/memory/README.mdtemplates/memory/Routines.md
- Lower SHRINK_FLOOR_BYTES from 1300 to 1250 so single-entry Routines files (~1270 B) are guarded by the catastrophic-shrink check - Add typeof guard to entryPolicyFromFrontmatter for defense-in-depth - Fix memory-review prompt to mention vault_delete_memory alongside vault_update_memory (step 6 introduces deletion but the final instruction only mentioned updates) - Fix "Active-commitments" → "Active commitments" hyphenation to match section heading (template + MEMORY_TEMPLATE_SPECS) - Fix "highest-value always-read" → "always-read file" (missing noun) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Responding to the two Overall Comments from the Sourcery review: 1. Derive SHRINK_FLOOR_BYTES at runtime from templates The threshold is deliberately a static constant measured against the rendered (bootstrapped) template sizes, not the raw template files. Runtime derivation would add fs reads at import time and async init complexity for a value that changes only when templates are edited — and the comment documents which templates were measured, so drift is caught in review. The static approach is the right tradeoff. 2. Validate entryPolicyFromFrontmatter against an explicit union The conservative default (any non-"living" → append-only) is deliberate and documented: append-only forbids destructive maintenance, so it's the safe reading for unknown/misspelled/missing values. Surfacing invalid values to callers would add error handling for a non-error condition — an unknown policy should be treated conservatively, not rejected. That said, I did add a 🔍 ship-check · pr-monitor · Claude Opus 4.6 (1M context) |
…_memory Errors
The duplicate no-op contract is already documented in the opening
paragraph ("Idempotent — an exact duplicate is a no-op"), the Returns
section, and the idempotentHint annotation. The Errors bullet was
triple-redundancy placed in a misleading section — a "not an error"
note inside "Errors:" contradicts its own framing and contributes to
the Conciseness (Co=4) TDQS ding.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The PR replaced the generic "H2 sections by theme/topic/cadence" lines with specific heading names (e.g. "Identity, Interests, Context"). These will go stale as agents add new H2 sections — vault_list_memory_files already surfaces the live heading structure, so the scope callout should describe the pattern, not enumerate the headings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
aliasunder
changed the title
aliasunder
changed the title
…onciliation, heading levels - Shrink guard floor: 200 B → 1250 B (drifted when PR #307 raised SHRINK_FLOOR_BYTES; doc now also states why the threshold sits just above the largest empty template) - Layer 2 auth attribution: requireBearerAuth is applied in mcp-core/mcp-router.ts, not server.ts - README leading-callouts claim reconciled with the opt-in include_leading_callout semantics on vault_search - Secure-by-default constraint scoped to the client-facing endpoint — the doc itself documents the default plaintext gateway→instance hop - Hybrid Search subsections promoted #### → ### to match every other h2's subsection level (slugs unchanged; no inbound links affected) - Capabilities base surface now lists property discovery + daily notes - One-vault constraint names both currency mechanisms (bind mount locally, Obsidian Sync remotely) - DOCKERHUB.md regenerated (table padding normalization only) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
What
Memory files gain a machine-readable
entry-policyfrontmatter property —append-only(the default; an absent property means this) orliving(a current-state file whose expired entries are pruned rather than left as history) — and the bootstrap template set grows to six files with a new Agents template for agent-facing directives.Why
Append-only was previously an unconditional statement in tool descriptions and the templates README, with no way for a file (or a user-created memory file) to declare an exception. Current-state content — upcoming plans, active commitments — structurally can't be append-only: without pruning, expired plans mislead every agent that reads them. Separately, lived-in memory layers drift toward mixing directives for agents with facts about the user; the Agents template gives directives a first-class home from day one, with the routing test documented in the templates README (who is the subject of the entry?).
Changes
memory-store.ts—MemoryEntryPolicytype;MemoryFileOutlinegainsentry_policy(unrecognized values resolve toappend-only, the safe reading); template specs gain the Agents file, per-spec policy, section-enumerating scope callouts, and cross-filerelatedlinks; programmatically-created files default toappend-only; shrink-guard floor recalibrated 200 → 1300 B to stay above the new largest empty template (Routines 1228 B)memory-tools.ts—vault_update_memory/vault_delete_memorydescriptions state the default and the living exception;vault_list_memory_filesdocuments the newentry_policyfieldmemory-review-prompt.ts— structural overview labels each file's policy; new reflection step 6 scoped to living files only (propose pruning expired entries); the never-prune contract now explicitly binds append-only filestemplates/memory/— newAgents.md; all templates declareentry-policyand enumerate sections; Routines restructured as the living example (Active commitments / Upcoming / Daily-weekly rhythm / Recent past); README documents both conventions.devin/wiki.jsonprompt purpose updatedlistMemoryFilescovers default/living/unrecognized policy values; prompt exact-output updated + a living-file overview test; tool-definition descriptions assert the entry-policy contractNo new tools — counts across README/server.json unchanged.
Notes for review
vault_memory_recallwork (Phase 3) plans to hydrateentry_policyinto recall results at query time; whichever branch lands second reconciles README/ARCHITECTURE touch points.Verification
npm test— 1644/1644 across 50 filesnpm run lint— 0 errorsnpm run build— clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation