feat: team-memory layer#257
Conversation
Adds a project-shared, version-controlled memory layer alongside the existing per-user and per-project memory. - New `Scope` enum (User / Project / Team) and `author` / `created_at` frontmatter fields on `MemoryMeta`. - Loader merges team-memory entries into the in-context memory with precedence project > team > user; collisions log a debug note. - New writer entry points `write_team_memory`, `list_team_memory`, and `delete_team_memory`. Writes are append-only by default; a collision without `force` returns an error. - `is_team_memory_path` guard predicate; background extraction skips any tool call that targets a team-memory path so the model can read team memory but cannot silently write to it. - New `/team-remember` slash command (with `list` and `remove` subcommands and `--force`) — the only sanctioned write path. Prompts for confirmation since entries enter version control. - Consolidation pipeline gains a `run_team_consolidation` wrapper that targets the team-memory dir. - Docs: extends `docs/concepts/memory.mdx` (and the mdBook copy) with a Team memory section, the read-but-not-silently-write invariant, and the precedence rule. Tests: round-trip load of a team-memory entry; collision precedence; that the per-user extraction dir cannot satisfy the team-memory path guard; team-write refuses overwrites without `--force` and accepts them with `--force`.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd5a53d673
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name_or_filename.to_string() | ||
| } else { | ||
| format!("{name_or_filename}.md") | ||
| }; | ||
| delete_memory(team_memory_dir, &filename) |
There was a problem hiding this comment.
delete_team_memory forwards unvalidated user input directly into delete_memory, which does team_memory_dir.join(filename); names like ../../README therefore resolve outside .agent/team-memory and allow /team-remember remove ... to delete arbitrary .md files in parent directories. Because this command is the new public delete path for team memory, it should reject path separators/.. segments (or canonicalize and enforce containment) before calling delete_memory.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
| } | ||
| by_id.into_values().collect() |
There was a problem hiding this comment.
Collecting merged entries via HashMap::into_values() produces non-deterministic iteration order, so the memory_files sequence (and therefore system prompt layout) can shuffle between loads even when inputs are unchanged. Since MemoryContext::load runs during prompt construction, this can cause prompt churn and unnecessary cache misses; the merged result should be made stable (e.g., preserve input order or sort by key/path before returning).
Useful? React with 👍 / 👎.
- delete_team_memory and write_team_memory now validate filenames (no path separators, no '..', no NUL/control chars, ASCII-printable, length-capped) and verify the resolved path stays inside the team-memory directory via canonicalize() as defense-in-depth. The same checks apply to the per-user write_memory and delete_memory paths so '/team-remember remove ../README' (and equivalents) can no longer delete arbitrary files. - merge_scoped_files used HashMap::into_values, whose iteration order is non-deterministic; the resulting memory_files Vec (and the system-prompt layout) shuffled between loads. Switched to BTreeMap and a final sort by (scope_priority, path) with User < Team < Project so prompt-cache hits stay stable. Added round-trip and order tests.
|
Addressed codex review findings: validated team-memory filenames (rejects path separators, '..', NUL/control chars, non-ASCII, overlong names) on both create and delete paths plus a canonicalize() defense-in-depth check; switched merge_scoped_files from HashMap to BTreeMap + (scope_priority, path) sort so the system-prompt layout is byte-stable across loads. Added tests for traversal rejection and merge determinism. |
…en writer - Block all write tools (FileWrite/FileEdit/MultiEdit/NotebookEdit) and Bash redirection from writing under `<project_root>/.agent/team-memory/`. The `/team-remember` slash command is the only sanctioned writer; it bypasses the tool permission check by calling `write_team_memory` directly. Permission checker grows an optional project root pinned at startup; when set, canonical-prefix containment refuses traversal. - Route consolidation `delete` actions through `delete_memory` so the validator + `ensure_path_within` guards apply. A consolidation reply with `../../README.md` no longer unlinks files outside the memory dir. - Validate `MEMORY.md` link targets the same way as the writer: refuse absolute paths, `..` segments, escapes from base_dir, and symlink leaves. Applies to user, team, and project memory. - Filename validator now rejects case-insensitive collisions with `MEMORY.md` / `README.md`, trailing dots/whitespace, and Windows- reserved device names (CON, PRN, AUX, NUL, COM1-9, LPT1-9). - `ensure_path_within` refuses a symlink leaf so `std::fs::write` can no longer follow a pre-planted link out of the memory dir. - `rebuild_index` sorts headers lexicographically before emitting so the on-disk MEMORY.md is byte-stable across mtime drift between checkouts.
|
Addressed second-round codex findings: enforce team-memory read-only invariant in the permission checker (writes via FileWrite/FileEdit/MultiEdit/NotebookEdit/Bash redirection are refused; |
1 participant
Summary
Adds Phase 8.11 from
ROADMAP.md: a project-shared, version-controlled memory layer that lives alongside per-user and per-project memory.<project>/.agent/team-memory/— same file format as user memory (frontmatter + body), so the existing reader handles it without changes. Frontmatter gains optionalauthorandcreated_atfields.Scopeenum (User/Project/Team) tagged on every loadedMemoryFile./team-rememberslash command — the only sanctioned write path. Prompts for confirmation since entries enter version control. Subcommands:list,remove <name>. Writes are append-only by default; collisions require--force. Each entry stamps the author fromgit config user.email(falls back tounknown) plus an ISO-8601 timestamp.is_team_memory_pathpredicate; background extraction now refuses to coast off anyFileWrite/FileEditthat targeted a team-memory path. The extraction loop's own writer continues to target only the per-user config dir, so the model can read team memory but not silently write to it.run_team_consolidationwrapper; the existing pipeline already takes a&Path, so consolidation on team-memory is a one-line call.docs/concepts/memory.mdxand the mdBook copy atdocs/src/concepts/memory.mdwith a Team memory section, the read-but-not-silently-write invariant, and the collision precedence rule.Test plan
cargo build --workspaceclean.cargo test -p agent-code-lib --lib— 789 passed (was 783; six new memory tests).cargo test -p agent-code(CLI unit + smoke) — green.cargo fmt --checkclean;cargo clippy --workspace --no-depsclean.MemoryContext::load, assert it appears withScope::Teamand the body roundtrips.ensure_memory_dir()(the only path background extraction writes to) does not satisfyis_team_memory_path.--forcereturns Err and leaves the file untouched;--forceoverwrites.is_team_memory_pathrecognizes.agent/team-memory/and rejects.agent/memory/.list_team_memoryskips theMEMORY.mdindex.delete_team_memoryaccepts a bare name without.md.gitfor the author lookup./team-remember test entry, confirm prompt fires, file lands under<project>/.agent/team-memory/, restart session, confirm the entry shows up under# Team Memory Indexin the system prompt section.