Expected Behavior
memory_update documents that "Only non-null arguments are applied." Updating tags or confidence on a memory should leave repo alone, so a memory filed under one repo stays discoverable from that repo's sessions.
Current Behavior
Against the deployed service at https://witan.ci.ol.mit.edu/mcp, any memory_update call that omits repo silently rewrites repo to the repo detected from the caller's working directory. Because memory_search and memory_list are repo-scoped by default, the memory then disappears from reads in the repo it actually documents, with no error and no warning.
Reproduced twice, on two memories, in two different fields, with the before-state captured from the deployment first. I was working in a checkout of mitodl/ol-infrastructure both times:
| memory |
field passed |
repo before |
repo after |
les-free-enrollment-500s-with-assertionerror-create--d14b24 |
tags only |
https://github.com/mitodl/mit-learn |
https://github.com/mitodl/ol-infrastructure |
pat-no-priority-tags-or-story-points-on-non-epic-iss-16acc7 |
confidence only |
https://github.com/mitodl/mit-learn |
https://github.com/mitodl/ol-infrastructure |
Worth flagging clearly: the code on main looks correct, so this is likely a deployed-version drift rather than a live defect in main, and the first step is probably to confirm what image CI is running.
memory_update builds changes under if value is not None, so an omitted repo never enters the dict.
_update_memory then merges "repo": changes.get("repo", current.get("repo")), preserving the stored value.
That handling dates to #170 and has not changed since, so the running deployment differs from main. The observed behavior matches an older _update_memory that resolved the repo through repo_module.detect(override=repo), which is what _store_memory still does correctly for new memories.
Steps to Reproduce
- Point the CLI and MCP server at a deployment (
remote_url set, witan login done).
- From a checkout of repo A, pick a memory whose
repo is repo B. Record its current repo via memory_list or witan memory --kind … --all-repos.
- Call
memory_update(slug=<that slug>, confidence=<its existing value>), passing no repo.
- Read the returned node.
repo is now repo A.
- The memory no longer appears in repo B's default-scoped
memory_search / memory_list.
Possible Solution
Confirm which witan image the deployments are running and roll them to a build that includes the _update_memory merge behavior on main.
Then add a regression test that a memory_update touching only a non-repo field leaves repo unchanged, ideally exercised through the remote path rather than local stdio, since local stdio detects the same repo the memory usually belongs to and would mask this.
Additional Details
Impact is higher than it first looks because the failure is silent and self-concealing. The natural time to edit a memory's tags or confidence is while working in some other repo, which is precisely when the rewrite fires. Nothing surfaces an error, and the memory simply stops appearing in the repo whose sessions need it.
Mitigation until the deployments are rolled: always pass repo explicitly on every memory_update call, even when you do not intend to change it.
This is closely related to #142, which fixed a different way repo keys fragmented (case folding, and detect(override=) bypassing normalise()). Same underlying hazard: a repo key that silently stops matching what repo.detect() returns.
I hit this while auditing a freshly migrated store, restored all affected rows by passing repo explicitly, and verified zero residual drift across all 14 of my memories against the original omnigraph export.
Expected Behavior
memory_updatedocuments that "Only non-null arguments are applied." Updatingtagsorconfidenceon a memory should leaverepoalone, so a memory filed under one repo stays discoverable from that repo's sessions.Current Behavior
Against the deployed service at
https://witan.ci.ol.mit.edu/mcp, anymemory_updatecall that omitsreposilently rewritesrepoto the repo detected from the caller's working directory. Becausememory_searchandmemory_listare repo-scoped by default, the memory then disappears from reads in the repo it actually documents, with no error and no warning.Reproduced twice, on two memories, in two different fields, with the before-state captured from the deployment first. I was working in a checkout of
mitodl/ol-infrastructureboth times:repobeforerepoafterles-free-enrollment-500s-with-assertionerror-create--d14b24tagsonlyhttps://github.com/mitodl/mit-learnhttps://github.com/mitodl/ol-infrastructurepat-no-priority-tags-or-story-points-on-non-epic-iss-16acc7confidenceonlyhttps://github.com/mitodl/mit-learnhttps://github.com/mitodl/ol-infrastructureWorth flagging clearly: the code on
mainlooks correct, so this is likely a deployed-version drift rather than a live defect inmain, and the first step is probably to confirm what image CI is running.memory_updatebuildschangesunderif value is not None, so an omittedreponever enters the dict._update_memorythen merges"repo": changes.get("repo", current.get("repo")), preserving the stored value.That handling dates to #170 and has not changed since, so the running deployment differs from
main. The observed behavior matches an older_update_memorythat resolved the repo throughrepo_module.detect(override=repo), which is what_store_memorystill does correctly for new memories.Steps to Reproduce
remote_urlset,witan logindone).repois repo B. Record its currentrepoviamemory_listorwitan memory --kind … --all-repos.memory_update(slug=<that slug>, confidence=<its existing value>), passing norepo.repois now repo A.memory_search/memory_list.Possible Solution
Confirm which witan image the deployments are running and roll them to a build that includes the
_update_memorymerge behavior onmain.Then add a regression test that a
memory_updatetouching only a non-repofield leavesrepounchanged, ideally exercised through the remote path rather than local stdio, since local stdio detects the same repo the memory usually belongs to and would mask this.Additional Details
Impact is higher than it first looks because the failure is silent and self-concealing. The natural time to edit a memory's tags or confidence is while working in some other repo, which is precisely when the rewrite fires. Nothing surfaces an error, and the memory simply stops appearing in the repo whose sessions need it.
Mitigation until the deployments are rolled: always pass
repoexplicitly on everymemory_updatecall, even when you do not intend to change it.This is closely related to #142, which fixed a different way repo keys fragmented (case folding, and
detect(override=)bypassingnormalise()). Same underlying hazard: a repo key that silently stops matching whatrepo.detect()returns.I hit this while auditing a freshly migrated store, restored all affected rows by passing
repoexplicitly, and verified zero residual drift across all 14 of my memories against the originalomnigraph export.