Skip to content

Fix swapped args in share_cube_with_user → _validate_cube_access (fixes #1901)#1902

Open
egouilliard-leyton wants to merge 2 commits into
MemTensor:mainfrom
egouilliard-leyton:fix/share-cube-with-user-args
Open

Fix swapped args in share_cube_with_user → _validate_cube_access (fixes #1901)#1902
egouilliard-leyton wants to merge 2 commits into
MemTensor:mainfrom
egouilliard-leyton:fix/share-cube-with-user-args

Conversation

@egouilliard-leyton

Copy link
Copy Markdown

Summary

Fixes #1901.

`MOSCore.share_cube_with_user(cube_id, target_user_id)` is currently unusable: every well-formed call raises `ValueError("User '<cube_id>' does not exist or is inactive")` because the args passed to `_validate_cube_access` are in the wrong order.

This one-line fix passes the current user (`self.user_id`) plus the `cube_id`, matching both the validator's signature `_validate_cube_access(self, user_id, cube_id)` and the in-code comment immediately above the call ("Validate current user has access to this cube"). The target user's existence is already validated on the next line.

Diff

```diff
def share_cube_with_user(self, cube_id: str, target_user_id: str) -> bool:
...
# Validate current user has access to this cube

  • self._validate_cube_access(cube_id, target_user_id)
  • self._validate_cube_access(self.user_id, cube_id)

    Validate target user exists

    if not self.user_manager.validate_user(target_user_id):
    raise ValueError(...)

    return self.user_manager.add_user_to_cube(target_user_id, cube_id)
    ```

Test plan

  • Add a `test_share_cube_with_user_success` unit test: owner creates a cube, adds a memory, shares with target, target's `search()` then returns the memory. (I can push this in a follow-up commit if a maintainer requests — the existing test layout in the repo would dictate the right location.)
  • Add a `test_share_cube_with_user_denies_non_owner`: a user who does not own/have access to the cube cannot share it.
  • Manual repro of the bug exists in the linked issue; the fix makes that repro pass.

Notes

  • Reproduces deterministically on `b60616d` (HEAD of `main`, v2.0.18-dev) and `MemoryOS==2.0.17` on PyPI.
  • I have not added a regression test in this PR because I wasn't sure which test directory layout the project prefers for `MOSCore` API tests. Happy to add one in a follow-up commit if a maintainer points to the right place.

share_cube_with_user(cube_id, target_user_id) passed args to
_validate_cube_access(user_id, cube_id) in the wrong order, raising
"User '<cube_id>' does not exist" for every well-formed call.

Per the in-code comment ("Validate current user has access to this
cube"), the validation should check the *current* (sharing) user
has access -- not the target -- so the fix uses self.user_id instead
of target_user_id. The target's existence is already validated on
the next line via user_manager.validate_user(target_user_id).

Fixes MemTensor#1901
@Memtensor-AI Memtensor-AI changed the base branch from main to dev-20260604-v2.0.19 June 10, 2026 15:43
@Memtensor-AI Memtensor-AI changed the base branch from dev-20260604-v2.0.19 to dev-v2.0.22 July 1, 2026 13:15
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Cloud AutoDev retest on dev-v2.0.22: NO EFFECTIVE DIFF.

Run: tr-f3d0be52-94d on test-engine-v3.
Result: fail_no_scope because changed files were none after merging dev-v2.0.22.

Recommendation: treat this PR as superseded/no-op rather than merge it.

@CarltonXiang CarltonXiang deleted the branch MemTensor:main July 3, 2026 07:25
@syzsunshine219 syzsunshine219 reopened this Jul 3, 2026
@syzsunshine219 syzsunshine219 added the needs-audit Requires manual audit before merge label Jul 3, 2026
@syzsunshine219 syzsunshine219 changed the base branch from dev-v2.0.22 to main July 3, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-audit Requires manual audit before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

share_cube_with_user passes swapped args to _validate_cube_access — fails for every well-formed call

5 participants