refactor: back persistent memory with pydantic-ai-harness - #187
refactor: back persistent memory with pydantic-ai-harness#187OchnikBartek wants to merge 3 commits into
Conversation
Coverage Report for CI Build 30259077581Coverage remained the same at 100.0%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
DEENUU1
left a comment
DEENUU1
left a comment
There was a problem hiding this comment.
This is a strong piece of work. Moving memory onto the harness store was the easy half; noticing that the move silently dropped per-tenant scoping and fork isolation, and then rebuilding both, is the half most migrations skip. BranchMemoryStore is the part I like most — seeding a path into a private overlay so CAS versions stay branch-local, and surfacing divergence once at flush time instead of failing a branch write mid-run, is exactly the right trade-off, and the docstring explains why rather than what. The eager mkdir with an actionable error message is also a good call: injection_errors="ignore" would otherwise have turned a bad memory_dir into an agent that quietly has no memory. And thank you for the "Notes for Reviewers" section — it made this review much faster.
Two things I'd like fixed before merge, both verified against harness 0.10.0 on this branch:
sanitize_agent_namedoesn't mirror the harness rule it says it mirrors — it misses the'..' not in segmenthalf, so a subagent namedv1..v2still passes the sanitizer and then raisesValueError: invalid memory pathat the first delegation. That's the exact run-time failure the function exists to prevent.BranchMemoryStore._seedis a check-then-act across anawaitwith no lock. Two memory tool calls in one model turn run concurrently in pydantic-ai (_tool_execution.pyonly serialisessequential=Truetools, and the harness memory tools aren't marked), so inside amemory="copy"branch the second one getsMemoryConflictError. I reproduced it with two concurrentread()calls.
The rest are non-blocking. Three more things that aren't anchored to diff lines:
- No CHANGELOG entry. Every recent PR here touches
CHANGELOG.md, and this one changes whatmemory_dirmeans — old values now raise, and existing on-diskMEMORY.mdfiles aren't migrated. That needs to be written down where upgraders will see it, not only in the PR body. docs/c4/4b-deep-dive-toolsets-and-capabilities.mdis stale. Lines 296-315 and 470-479 still documentupdate_memory,memory_dir = /.deep/memoryas the default, and system-prompt injection. You swept c4 pages 1 and 2 but not 4b, so a reader lands on a documented default that now raises.pydantic_deep/agent.py:1303buildsImproveToolset(working_dir=Path("."))while the CLI passesmemory_base_dir=str(root). Even with thememory_dirfix below, improve and the agent only agree when the process CWD happens to equal the backend root. See the analyzer comment.
| if _VALID_AGENT_NAME_RE.fullmatch(agent_name): | ||
| return agent_name | ||
| cleaned = re.sub(r"[^A-Za-z0-9_.-]+", "-", agent_name).strip("-")[:200] | ||
| return cleaned or "agent" |
There was a problem hiding this comment.
The docstring says this mirrors the harness' _VALID_SEGMENT_RE, but the harness check is two conditions, not one:
# pydantic_ai_harness/memory/_store.py:83
if not all(_VALID_SEGMENT_RE.fullmatch(segment) and '..' not in segment for segment in path.split('/')):.. is missing here, and . is inside the character class, so a name containing .. sails through the fast path unchanged and then blows up exactly where this function was written to stop it. I ran it on this branch:
'..' -> sanitized '..' ValueError: invalid memory path: '..'
'a..b' -> sanitized 'a..b' ValueError: invalid memory path: 'a..b'
'v1..v2' -> sanitized 'v1..v2' ValueError: invalid memory path: 'v1..v2'
So a subagent named v1..v2 builds fine and dies on first delegation — the same run-time failure as code reviewer, just a different character. With a FileStore it's also the path-escape case, which is why the harness rejects it.
| if _VALID_AGENT_NAME_RE.fullmatch(agent_name): | |
| return agent_name | |
| cleaned = re.sub(r"[^A-Za-z0-9_.-]+", "-", agent_name).strip("-")[:200] | |
| return cleaned or "agent" | |
| if _VALID_AGENT_NAME_RE.fullmatch(agent_name) and ".." not in agent_name: | |
| return agent_name | |
| cleaned = re.sub(r"[^A-Za-z0-9_.-]+", "-", agent_name) | |
| cleaned = re.sub(r"\.{2,}", ".", cleaned).strip("-.")[:200] | |
| return cleaned or "agent" |
I checked this against validate_store_path for .., a..b, v1..v2, ..., ..a.., code reviewer, ???, a 250-char name and the ordinary main/reviewer cases — all pass, and already-valid names are still returned unchanged.
One leftover I left out of the suggestion because it's marginal: a bare "." still survives the fast path, and under FileStore _resolve("./MEMORY.md") realpaths to the store root, so that agent's memory lands beside the other agents' directories instead of inside its own. Your call whether that's worth a third condition.
| async def _seed(self, path: str) -> None: | ||
| """Copy `path` from the parent into the overlay, once.""" | ||
| if path in self._seeded: | ||
| return | ||
| parent_file = await self._parent.read(path, max_chars=_SEED_MAX_CHARS) | ||
| if parent_file is None: | ||
| self._seeded[path] = None | ||
| return | ||
| if parent_file.truncated: # pragma: no cover - guards against a silent partial copy | ||
| raise RuntimeError( | ||
| f"branch memory seeding read a truncated {path!r}; refusing to stage a " | ||
| "partial copy that flush_to would write back over the parent" | ||
| ) | ||
| self._seeded[path] = parent_file.version | ||
| await self._overlay.write(path, parent_file.content, expected_version=None) |
There was a problem hiding this comment.
This is a check-then-act across an await with nothing guarding it. Two coroutines both find path not in self._seeded, both await self._parent.read(...), and the second self._overlay.write(..., expected_version=None) hits an overlay where the path now exists — InMemoryStore.write raises MemoryConflictError.
It's reachable: pydantic-ai runs tool calls from one model response in parallel (_tool_execution.py only segments on sequential=True tools, and the harness MemoryToolset registers its four functions with the default), so any turn inside a memory="copy" branch that emits e.g. search_memory + read_memory — search seeds every parent path under the prefix, read seeds one of them — races. Reproduced on this branch with the simplest version:
b = BranchMemoryStore(parent)
await asyncio.gather(b.read(p, max_chars=1000), b.read(p, max_chars=1000), return_exceptions=True)
# -> [MemoryFile, MemoryConflictError: memory path 'main/MEMORY.md' changed before it could be written]A single lock is enough — seeding is short, and InMemoryStore already does the same thing. Add import anyio at the top and self._seed_lock = anyio.Lock() in __init__ next to self._overlay, then:
| async def _seed(self, path: str) -> None: | |
| """Copy `path` from the parent into the overlay, once.""" | |
| if path in self._seeded: | |
| return | |
| parent_file = await self._parent.read(path, max_chars=_SEED_MAX_CHARS) | |
| if parent_file is None: | |
| self._seeded[path] = None | |
| return | |
| if parent_file.truncated: # pragma: no cover - guards against a silent partial copy | |
| raise RuntimeError( | |
| f"branch memory seeding read a truncated {path!r}; refusing to stage a " | |
| "partial copy that flush_to would write back over the parent" | |
| ) | |
| self._seeded[path] = parent_file.version | |
| await self._overlay.write(path, parent_file.content, expected_version=None) | |
| async def _seed(self, path: str) -> None: | |
| """Copy `path` from the parent into the overlay, once.""" | |
| async with self._seed_lock: | |
| if path in self._seeded: | |
| return | |
| parent_file = await self._parent.read(path, max_chars=_SEED_MAX_CHARS) | |
| if parent_file is None: | |
| self._seeded[path] = None | |
| return | |
| if parent_file.truncated: # pragma: no cover - guards against a silent partial copy | |
| raise RuntimeError( | |
| f"branch memory seeding read a truncated {path!r}; refusing to stage a " | |
| "partial copy that flush_to would write back over the parent" | |
| ) | |
| self._seeded[path] = parent_file.version | |
| await self._overlay.write(path, parent_file.content, expected_version=None) |
Worth a test that gathers two concurrent branch reads of the same path — tests/test_forking_memory.py covers the sequential paths thoroughly but nothing exercises two at once.
| winner_memory = getattr(winner.deps, "memory_store", None) | ||
| if isinstance(winner_memory, BranchMemoryStore): | ||
| memory_report = await winner_memory.flush_to() | ||
| memory_conflicts = list(memory_report.conflicts) |
There was a problem hiding this comment.
The file overlay's conflicts go into MergeResult.conflicts and its errors into MergeResult.errors, but a memory conflict only reaches a log line. Same concept, two places, and only one of them is visible to the caller — so an app that checks if result.conflicts: ... to decide whether the merge was clean will report a clean merge while the winner's write_memory was silently dropped.
I'd fold them into the existing fields so there's one thing to check:
winner_memory = getattr(winner.deps, "memory_store", None)
memory_conflicts: list[str] = []
if isinstance(winner_memory, BranchMemoryStore):
memory_report = await winner_memory.flush_to()
memory_conflicts = [f"memory:{path}" for path in memory_report.conflicts]
if memory_conflicts:
logger.warning(...)and then conflicts=[*conflicts, *memory_conflicts] in the MergeResult. The memory: prefix keeps them distinguishable from backend paths without needing a new field. If you'd rather keep the two surfaces separate, a memory_conflicts: list[str] field on MergeResult works too — the thing I don't want is the caller having to read logs to find out.
| working_dir: Path | None = None, | ||
| on_progress: ProgressCallback | None = None, | ||
| context_files: dict[str, str] | None = None, | ||
| memory_dir: str | None = None, |
There was a problem hiding this comment.
This parameter is never passed by anything in the library. ImproveToolset.__init__ doesn't take a memory_dir, and create_deep_agent builds it at pydantic_deep/agent.py:1303 without one, so the analyzer always falls back to DEFAULT_IMPROVE_MEMORY_DIR.
That means the alignment fix holds only for the default. create_deep_agent(memory_dir="~/.myapp/memory", include_improve=True) puts the agent's FileStore under ~/.myapp/memory/main/MEMORY.md while improve still writes {working_dir}/.pydantic-deep/main/MEMORY.md — which is the same desync TestImproveMemoryAlignment says it's guarding, just moved from the directory name to the directory the caller chose. The four new tests all construct ImprovementAnalyzer directly, so none of them would catch it.
Threading it through is small:
# improve/toolset.py
class ImproveToolset(FunctionToolset[Any]):
def __init__(self, ..., memory_dir: str | None = None) -> None:
self._memory_dir = memory_dir
# ...and pass memory_dir=self._memory_dir to both ImprovementAnalyzer(...) calls
# agent.py:1303
improve_toolset = ImproveToolset(
sessions_dir=_improve_sessions,
working_dir=Path("."),
model=model if isinstance(model, str) else DEFAULT_IMPROVE_MODEL,
memory_dir=memory_dir,
)While you're in there: working_dir=Path(".") is the other half of the same alignment. The CLI now passes memory_base_dir=str(root), so improve and the agent agree only when the process CWD equals the backend root — start the CLI with --working-dir pointing elsewhere and they diverge again. _wd is already computed a few lines above for sessions_dir; using it for working_dir too would close it.
Smaller thing in this constructor: self._context_files = context_files or dict(DEFAULT_CONTEXT_FILES) treats {} as "use defaults", but the guard below is if context_files is None, so context_files={} gets the defaults and ignores memory_dir. if context_files is None in both places would make it consistent.
| else: | ||
| # The default factory attaches memory as a capability; a caller- | ||
| # supplied factory/agent never runs it, so wire the memory tools | ||
| # in through `toolsets` the way the pre-capability code did. |
There was a problem hiding this comment.
"the way the pre-capability code did" isn't quite true, and the gap is worth spelling out because it's a silent behaviour change.
AgentMemoryToolset had a get_instructions() that loaded the first N lines of MEMORY.md into the prompt, so the old toolset-injection path gave a subagent both the tools and its memory. The harness splits those: MemoryToolset is tools only, and the loading happens in Memory.before_model_request plus the guidance in Memory.get_instructions() — neither of which runs for a capability that was never registered on the agent.
So a subagent with its own agent_factory now gets four memory tools, no memory in its context, and no guidance telling it the tools exist. It'll only ever see its memory if it independently decides to call read_memory.
I don't think there's a clean fix — SubAgentConfig has no capabilities field, and agent_kwargs isn't consulted on the custom-factory path — so the toolset fallback is the right call. I'd just make the docstring say what it actually delivers, something like:
Subagents built by the default factory receive memory as a `Memory`
capability. Those carrying their own `agent_factory` (or a prebuilt
`agent`) never reach it, so the tools are wired in through `toolsets`
instead. Note this gives tools only: `Memory.before_model_request` never
runs for an unregistered capability, so memory is not auto-injected into
the subagent's context and the usage guidance is not added — a caller who
wants that must register the capability on the agent they build.
Worth a line in docs/learn/memory.md too, since it's the kind of thing someone debugs for an hour.
| for sa_config in effective_subagents: | ||
| if ( | ||
| sa_config.get("agent") is None and sa_config.get("agent_factory") is None | ||
| ): # pragma: no branch |
There was a problem hiding this comment.
nit: this # pragma: no branch is stale now. It says the false branch is never taken, but you added an else and test_custom_agent_factory_subagent_keeps_memory_tools exercises it, so the pragma is both a no-op and misleading to the next reader.
| ): # pragma: no branch | |
| ): |
DEENUU1
left a comment
DEENUU1
left a comment
There was a problem hiding this comment.
Second pass, this time on the parts I skimmed the first time round — the merge critical section, and the docs that tell people how to use the new per-tenant story. Everything below is reproduced on this branch against harness 0.10.0.
One more blocking bug and one blocking docs problem, both in the area the PR is about:
BranchMemoryStore.flush_tocan raise, and the coordinator doesn't catch it.BranchOverlay.flush_towas deliberately built the other way — its own docstring says "flush_tonever aborts on the first failure" and per-write failures land inFlushReport.errors. The memory flush runs right after it in the same critical section with no such guard, so one bad path takes the whole merge down after the backend already landed and every loser was already cancelled.docs/advanced/multi-user.md— the prose is right, but both runnable examples on the page leak memory between users. The page's own table says "If you skip it: Users see each other's memory", and then both code blocks skip it.
Three non-blocking, plus a couple of answers:
Your question — "worth checking no TUI/CLI preset sets memory=\"share\" by default." Nothing does. The only construction sites are coordinator.py:528 (isolation or BranchIsolation()) and forking/__init__.py:81-82, both landing on the copy default. The one way share gets set is the model itself, through the fork tool's isolation: dict[str, Any] argument.
Which surfaces a small related thing: _coerce_isolation does BranchIsolation(**raw) on that model-supplied dict, and a plain dataclass doesn't validate its Literals. An unknown key raises loudly, but a known key with a junk value is silent — and since every flag is tested with == "copy", {"memory": "isolated"} from a confused model quietly means share-with-parent, so branch writes hit the parent and survive a discarded branch. That's a pre-existing pattern (backend and todos have the same shape), but memory is the flag this PR just made load-bearing, so it's worth a BranchIsolation __post_init__ check or a Pydantic model at the tool boundary.
One more unwired cell for memory_path_for (on top of the memory_dir comment from my last pass): it doesn't know about memory_namespace either. With a namespace set, the agent reads {memory_dir}/{namespace}/main/MEMORY.md — Memory._resolve_scope builds f'{namespace}/{agent_name}' — while improve writes {memory_dir}/main/MEMORY.md. Whatever shape the memory_dir fix takes, the namespace needs to ride along with it.
| # the losers' are dropped with their deps. | ||
| winner_memory = getattr(winner.deps, "memory_store", None) | ||
| if isinstance(winner_memory, BranchMemoryStore): | ||
| memory_report = await winner_memory.flush_to() |
There was a problem hiding this comment.
This await can raise, and nothing here catches it. destination.read(...) and destination.write(...) inside flush_to can both throw — OSError from a FileStore, or MemoryConflictError if a third actor writes between the read and the CAS write a few lines later (the same check-then-act window the conflict detection is built around, just at the flush site instead).
The asymmetry is what makes it bite. BranchOverlay.flush_to right above collects per-write failures into report.errors and its docstring is explicit that it "never aborts on the first failure". The memory flush has no equivalent, so a single failing path escapes merge_or_select — and by then the backend overlay has already flushed to the parent and every loser has already been cancelled and dropped.
Repro on this branch (parent store whose write raises OSError, one branch, pick: merge):
winner.overlay still attached: True
fork resolved: False
So: winner.overlay = None never runs, MergeResult is never built, materializer.cleanup() never runs, _cached_outcome is never reset — and the losers are gone while the winner's files are on disk. The fork is stuck open in a state no caller can recover from, over a memory write.
Two halves to the fix. In BranchMemoryStore.flush_to, make the loop tolerant the way BranchOverlay is — add an errors: list[tuple[str, str]] field to MemoryFlushReport and wrap the per-path body:
for path in self._touched:
try:
... # existing read / conflict / delete / write
except Exception as exc:
report.errors.append((path, f"{type(exc).__name__}: {exc}"))And here, surface them next to the conflicts rather than letting anything escape the critical section. Worth a test with a parent store that raises, asserting the merge still completes and the failure is reported — the two TestMergeFlush cases cover the happy path and the conflict path, but nothing covers a store that misbehaves.
| !!! note "Memory has its own store" | ||
| Persistent memory no longer lives in the backend — it uses a pluggable | ||
| `MemoryStore`. It still follows `deps`, though: pass | ||
| `DeepAgentDeps(memory_store=...)` per request and that store wins over the one | ||
| the agent was built with, so a single shared agent stays isolated. Alternatively | ||
| give the agent a `memory_dir=`/`memory_store=` per user, or pass a per-tenant | ||
| `memory_namespace=` to partition one shared store. Checkpoints likewise use a | ||
| separate `checkpoint_store`. |
There was a problem hiding this comment.
This note is correct, and the table you added at line 131 is correct too ("Memory | DeepAgentDeps(memory_store=…) per run | If you skip it: Users see each other's memory"). But neither runnable example on the page does it, so the code a reader copies demonstrates exactly the leak the table warns about.
I checked, on a page titled "One agent, many users — without their state ever touching":
agent = create_deep_agent(model=..., memory_dir=d) # built once, as the page says
async def run_as(user):
deps = DeepAgentDeps() # exactly lines 30-32
await agent.run("hi", deps=deps)
return deps.memory_store
# alice store is bob store: TrueThat's deps_store_resolver doing its job — deps carry nothing, so it seeds them with the agent's shared store, and alice and bob end up in the same main/MEMORY.md.
Both examples need the extra line. Lines 30-32:
deps = DeepAgentDeps(
backend=LocalBackend(root_dir=f"/workspaces/{user_id}"), # (2)!
memory_store=FileStore(f"/memory/{user_id}"), # (3)!
)And the FastAPI one at lines 155-158, which matters more because it's the production-shaped one and it already scopes checkpoint_store per user — memory is the one thing it misses:
return DeepAgentDeps(
backend=LocalBackend(root_dir=f"/workspaces/{user_id}"),
memory_store=FileStore(f"/memory/{user_id}"),
checkpoint_store=FileCheckpointStore(f"/checkpoints/{user_id}"),
)Given restoring per-tenant scoping is half the point of this PR, I'd rather the page's headline example show it than describe it.
| memory_namespace: Per-tenant namespace for the harness `Memory` | ||
| capability, isolating users that share one store. Use this for the | ||
| build-agent-once / deps-per-user pattern, where a single store is | ||
| shared across requests. Defaults to "" (no namespace). |
There was a problem hiding this comment.
This advice can't be followed. memory_namespace: str is bound at agent construction, so "build the agent once" gives you exactly one namespace for every request — the opposite of per-tenant. To actually vary it per user you'd have to build one agent per user, which is the pattern this paragraph is telling you to avoid.
It's the same shape as the bug you fixed on this page's docs counterpart: pointing people at a namespace knob that doesn't do the job. Here the knob exists, it just isn't per-run.
The harness already supports it — Memory.namespace is str | Callable[[RunContext[AgentDepsT]], str] and _resolve_scope does self.namespace(ctx) if callable(self.namespace) else self.namespace. build_memory_capability narrows it to str on the way through, which is where it gets lost. Widening it is small:
# features/memory/store.py
def build_memory_capability(
*,
namespace: str | Callable[[RunContext[Any]], str] = "",
...plus the same union on memory_namespace in the three create_deep_agent signatures and in _make_default_deep_agent_factory (it already forwards the value to subagents, so they'd follow the tenant too). DeepAgentSpec.memory_namespace stays str — a callable isn't serializable, and the YAML case is genuinely the static one.
Then memory_namespace=lambda ctx: ctx.deps.user_id becomes the answer for build-once/deps-per-user, and the docstring is true as written. If you'd rather not widen it, the fix is the other direction: say plainly that memory_namespace is for one-tenant-per-agent, and point the build-once pattern at DeepAgentDeps(memory_store=...) instead.
| # Memory used to always persist (backend `/.deep/memory`). It now | ||
| # persists only when the caller says where, so an unconfigured agent | ||
| # would silently drop everything it "remembered" at process exit. | ||
| if not memory_dir: |
There was a problem hiding this comment.
The 438 warnings you flagged in the suite aren't only a test-noise problem — this fires on the default constructor, so it fires for essentially every user and every documented example. include_memory=True and memory_dir=None are both defaults, so the README's headline quickstart
agent = create_deep_agent(model="anthropic:claude-sonnet-4-6")now prints a paragraph of UserWarning (I confirmed it). There are ~271 create_deep_agent( call sites across docs/ and the README and only a handful pass anything memory-related, so "the library warns you on the happy path" is the new default experience.
The warning itself is the right instinct — silently losing memory at process exit is worth shouting about. The problem is it can't distinguish "I asked for memory and forgot to say where" from "I never thought about memory and got it switched on". A sentinel separates them, and it's a pattern already used in this codebase (apps/cli/agent.py:162 does include_improve: bool | None = None):
include_memory: bool | None = None, # None = default-on, quietly ephemeral
...
_include_memory = True if include_memory is None else include_memory
if _include_memory and memory_store is None and not memory_dir and include_memory is True:
warnings.warn(...) # only when the caller explicitly opted inThat keeps the shout for the case that deserves it and takes it off the quickstart. Whichever way you go, the ~438-warning test suite should end up quiet too — a suite that warns 438 times is a suite where nobody reads warnings, which is how the next one gets missed.
| new_todos = list(deps.todos) if isolation.todos == "copy" else deps.todos | ||
|
|
||
| # "copy" → staged, discardable branch memory; "share" → the parent's store. | ||
| new_memory_store: Any = deps.memory_store |
There was a problem hiding this comment.
nit: the Any isn't needed here. BranchMemoryStore structurally satisfies the MemoryStore protocol — I checked both checkers:
mypy --strict : Success: no issues found in 1 source file
pyright : 0 errors, 0 warnings, 0 informations
and swapping the annotation in this file passes mypy --strict on the module as-is. The Any on raw_backend two lines up has a real reason (unwrap_backend returns Any); this one just turns off type checking on the value being assigned to a typed field, which is the thing the standards ask us not to do.
| new_memory_store: Any = deps.memory_store | |
| new_memory_store: MemoryStore | None = deps.memory_store |
Needs from pydantic_ai_harness.memory import MemoryStore at the top — the module already imports BranchMemoryStore from the sibling, so it's one more line next to it.
3 participants
Summary
Migrates persistent memory from pydantic-deep's own backend-backed implementation
to the upstream
pydantic-ai-harnessMemorycapability, and restores the twoisolation guarantees that move silently broke: per-tenant scoping and fork/branch
memory.
Added
pydantic_deep/features/memory/store.py— the single place mapping ouroptions onto a harness store:
build_memory_store,resolve_memory_dir,sanitize_agent_name,deps_store_resolver, and the sharedbuild_memory_capabilityused by both the main agent and subagents.pydantic_deep/features/forking/memory.pywithBranchMemoryStoreandMemoryFlushReport— a copy-on-writeMemoryStoreoverlay, the memorycounterpart of
BranchOverlay, plusflush_to()for merge-time replay.memory_storetoDeepAgentDeps(deps.py), so a per-run storeoverrides the one the agent was built with.
memory_namespace=andmemory_base_dir=tocreate_deep_agent(allthree signatures) and as
DeepAgentSpecfields inspec.py.pydantic-ai-harness>=0.10.0to base dependencies inpyproject.toml.There is no
[memory]extra — harness 0.10.0 ships memory in the base package.memory_dirtoImproveAnalyzer.__init__withmemory_path_for()andDEFAULT_IMPROVE_MEMORY_DIRinfeatures/improve/analyzer.py.Changed
Memorycapability fromcreate_deep_agent(include_memory=True)instead of
AgentMemoryToolset. Memory is now injected as user-role contextrather than a system-instruction block;
update_memoryis gone, replaced bywrite_memory(old_text=...), andsearch_memoryis new.MemoryCapability,AgentMemoryToolset,MemoryFile,load_memory,get_memory_path,format_memory_prompt, theDEFAULT_MEMORY_*/DEFAULT_PIN_END_MARKERconstants) — still importable,now emitting
DeprecationWarning, following thecapabilities/pattern.memory_diras a host filesystem path, not a path insidedeps.backend. Backend-only values like/.deep/memorynow raise aValueErrornaming the move instead of failing at run time with
OSError: [Errno 30] Read-only file system.memory_diragainstmemory_base_dir(default: processCWD), and made
apps/cli/agent.pypassmemory_base_dir=str(root)so memoryfollows
--working-dirand stays where/rememberwrites.agent.py:include_memory=Truewith no
memory_dir/memory_store(ephemeral store), and a hostFileStorepaired with a non-local backend.
Memorywith astore_resolver(deps_store_resolver) that prefersdeps.memory_storeand otherwise seeds it with the agent's store. Seeding iswhat gives
clone_for_brancha parent store to wrap, since the fork machineryclones
deps, not the agent.clone_for_branchreadBranchIsolation.memory, which was previously onlyrecorded:
"copy"wraps the parent store in aBranchMemoryStore,"share"passes it through.
ForkCoordinator._mergenow flushes the winning branch'sstaged memory and logs skipped conflicts; losers' writes are dropped with their
deps.
agent_namevalues before they reach the store. The harnessenforces
[A-Za-z0-9_.-]{1,200}viavalidate_store_pathon every run, so aname like
"code reviewer"previously built fine and raised at first delegation._inject_subagent_memory_toolsetinagent.pyfor subagents supplyingtheir own
agent_factoryor a prebuiltagent— those bypass the defaultfactory and had been left with
toolsets: [].extra={"memory": False}ishonoured there.
defer_loading=tool_searchon the memory capability. Riding thecapability list bypassed
defer_situational_toolsets, which only wrapsall_toolsets, so all four memory tool schemas were sent upfront..deep/memory/main/MEMORY.mdto
.pydantic-deep/main/MEMORY.md— the file the agent'sFileStoreactuallyreads.
extra={"memory_pin_marker": ...}into aDeprecationWarning. Theharness has no pin concept and its truncation keeps the tail of
MEMORY.md,so a pinned header would be the first thing dropped.
CLAUDE.mdand 13 docs pages, including thememory_dir="/.deep/memory"example indocs/concepts/agents.mdthat crashedas written, and the
docs/advanced/multi-user.mdadvice to use anamespaceparameter that did not exist.
Testing
Added
tests/test_forking_memory.py(23 tests): resolver precedence andseeding,
clone_for_branchper isolation flag, read-through, staged writes,discard leaving the parent untouched, flush apply/create/delete, conflict
detection, branch-local CAS surviving a concurrent parent write, merged
list_paths, search over not-yet-read parent content, branch-local operationreceipts, fork-of-fork propagation, and per-tenant non-leakage.
Added
TestMemoryStoreWiringtotests/test_memory.py(12 tests): backend-stylememory_dirraising, base-dir anchoring, name sanitization, the ephemeralwarning, namespace propagation, parametrized
tool_searchdeferral, thepin-marker warning, and custom-factory subagents keeping all four memory tools.
Added
TestImproveMemoryAlignmenttotests/test_improve.py(4 tests) pinningimprove's target to what the agent's store reads on disk — nothing guarded this,
which is why it regressed silently.
Updated
tests/test_agent.py,tests/test_improve.py,tests/test_spec.py,tests/test_eviction.pyandtests/test_processors.pyfor the new factorysignature, spec fields and memory paths.
Ran
pytest tests/— 2759 passed.ruff checkclean,mypy pydantic_deep/clean across 132 source files.
Drove the wiring end-to-end on
FunctionModelrather than only through unittests, confirming the resolver seeds deps at run start (with
tool_search=Truetoo), the branch wraps the parent, a discarded branch write never reaches the
parent, and a flush applies it:
Notes for Reviewers
deps_store_resolvermutatesctx.deps.memory_storewhen it is unset. Thisis deliberate — the fork machinery clones deps, so a branch needs a parent store
reachable from deps — and it mirrors what
LiveForkCapabilityalready does withctx.deps.fork_coordinator. Worth a look if you disagree with the precedent.BranchMemoryStoreseeds a path copy-on-first-touch into a privateInMemoryStoreso CAS versions are branch-local. Without that, a concurrentparent write would fail a branch write mid-run. The trade-off: a branch sees the
parent as of first touch, not later, and divergence surfaces once at flush as a
conflict rather than overwriting the newer parent.
MEMORY.mdfiles are not migrated. Combined withmemory_dirchanging meaning, an app upgrading across this patch bump needs tomove its memory directory or pass
memory_store=. The new warning covers theno-config case but cannot cover a caller who already passed a backend-style
memory_dir— that raises.memory="share"onBranchIsolationkeeps the old write-through behaviour asan explicit opt-in. Worth checking no TUI/CLI preset sets it by default.
UserWarningfires in existing tests (438 warnings inthe suite). Tests pass, but if the noise is unwanted the fix is either explicit
include_memory=False/memory_dir=in those tests or afilterwarningsentry— it touches many test files, so it is left out here.
pydantic-ai-todoontopydantic-ai-harness/planning/is out ofscope; the upstream contribution there is still in flight.