Skip to content

Conversation

@frederikb96
Copy link
Contributor

Problem

MCP search_memory endpoint crashed with TypeError: argument of type 'NoneType' is not iterable when filtering search results by ACL permissions.

Root cause: Boolean logic error in filter condition (line 187):

if allowed and h.id is None or h.id not in allowed:  # Wrong

This evaluates as (allowed and h.id is None) or (h.id not in allowed), causing crash when allowed is a non-empty set.

Fix

Corrected the boolean logic:

if allowed is not None and h.id not in allowed:  # Correct

Now properly checks: "If ACL filter exists AND memory ID not in allowed set, skip it"

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Test Script - MCP search now returns results without TypeError
  • Verified with populated memory database and ACL filtering

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings

Copy link
Contributor

@parshvadaftari parshvadaftari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@parshvadaftari
Copy link
Contributor

Good catch @frederikb96 thanks for pointing it out and fixing it.

@frederikb96
Copy link
Contributor Author

frederikb96 commented Oct 20, 2025

@parshvadaftari Thank you very much for starting to merge some of my PRs 🙂

Any chance that my other PRs are getting merged soon? 🙂

They are all mostly tiny atomic changes all around the OpenMemory sub project. There were many bugs and problems in it and a lot of room for improvements. Since I am using it on my fork now in production, I refactored all and created all those atomic PRs. I have like 10 more to go - some bigger once, too 😄 but they depend on those here. So I wait for the subsequent PRs until you decided if merging is fine or changes are needed, etc. 🙂

#3624
#3620
#3619
#3618
#3616
#3607
#3601

@parshvadaftari
Copy link
Contributor

Hey @frederikb96 thank you for the prs, our team is reviewing them. Thank for fixing the bugs, will drop comments on them 😄

Copy link
Contributor

@parshvadaftari parshvadaftari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

- Fix boolean logic in ACL filter check
- Change from 'if allowed and h.id is None or h.id not in allowed' to 'if allowed is not None and h.id not in allowed'
- Prevents TypeError when allowed set exists but is checked incorrectly
@frederikb96 frederikb96 force-pushed the fix/mcp-search-nonetype branch from 5ae0188 to a781e4c Compare October 31, 2025 08:06
@frederikb96
Copy link
Contributor Author

@parshvadaftari I rebased again on main, ready to merge? 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants