Fix: memory-context cache survives invalidation, leaking stale snapshots#1
Draft
mimeding wants to merge 2 commits into
Draft
Fix: memory-context cache survives invalidation, leaking stale snapshots#1mimeding wants to merge 2 commits into
mimeding wants to merge 2 commits into
Conversation
The assembler keyed cache entries on '(agentId, toolsAvailable)' but invalidateCache(agentId:) removed only the bare 'agentId' key. The two never matched, so stale 10-second snapshots survived user-visible memory edits and the chatOnly/withTools partition fix from PR osaurus-ai#877 could appear to regress on the next compose pass after window changes. Fix invalidateCache to drop both 'tools=0' and 'tools=1' entries for the given agent, and add regression tests using small internal test-only helpers (_seedCache / _hasCachedEntry) so the eviction contract is verifiable without standing up the full MemoryDatabase stack. Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
ModelManager.init kicks off an unstructured Task that calls loadOsaurusAIOrgModels(), which fetches the OsaurusAI organization listing from Hugging Face and feeds the result through applyOsaurusOrgFetch. The unit-test runner repeatedly constructs ModelManager() to drive applyOsaurusOrgFetch directly. The background launch-time fetch races with those test calls — whichever finishes last wins, and the merge result is non-deterministic. That's the root cause of the flaky ModelManagerSuggestedTests failures seen across many of the recent PR CI runs (applyOsaurusOrgFetch_dropsStaleAutoFetched OnReapply, applyOsaurusOrgFetch_addsNewEntriesAfterCurated, etc.). Gate the launch-time fetch on a small isRunningInTestEnvironment helper that checks for any of XCTestConfigurationFilePath, XCTestBundlePath, or XCTestSessionIdentifier in the process environment. Those variables are only present inside an xctest host process; production app launches still get the HF fetch exactly as before. This is a network call, so removing it under tests also has the side benefit of making the test suite work offline / on hermetic CI runners. Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why this matters (business)
Agents are supposed to react immediately when a user edits their memory (changing a fact, adding a user override, switching tool availability). With this bug, changes a user just made could silently fail to take effect for up to 10 seconds after a chat window event — the agent answers as if the edit never happened. That's exactly the kind of "did it work?" UX issue that erodes trust in local memory, and it can subtly regress the chat/tool partition fix that landed in osaurus-ai#877.
What's wrong (technical)
MemoryContextAssemblercaches assembled context with a composite key:…but
invalidateCache(agentId:)removed entries using only the bareagentId:The key never matched, so the call was a no-op. Callers that depend on this contract — notably
ChatWindowManageron window close — could not actually force a fresh assembly:The comment above describes the exact symptom the bug allows.
Fix
Evict both
tools=0andtools=1partitions for the givenagentId. The global (nil) and per-agent invalidation now both behave as advertised.Adds two test-only internal helpers (
_seedCache,_hasCachedEntry) so the eviction contract can be verified without standing upMemoryDatabase,MemorySearchService, and an embedder just to populate the cache through the normal path.Changes
Test Plan
cd Packages/OsaurusCore && swift test --filter MemoryContextAssemblerTestsChecklist
CONTRIBUTING.md