Skip to content

witan: memory_update silently rewrites repo to the caller's detected repo, re-scoping the memory out of its own repo #268

Description

@gumaerc

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

  1. Point the CLI and MCP server at a deployment (remote_url set, witan login done).
  2. 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.
  3. Call memory_update(slug=<that slug>, confidence=<its existing value>), passing no repo.
  4. Read the returned node. repo is now repo A.
  5. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions