diff --git a/src/scribe/services/access.py b/src/scribe/services/access.py index 2598215..bb39d61 100644 --- a/src/scribe/services/access.py +++ b/src/scribe/services/access.py @@ -209,16 +209,28 @@ def notes_visibility_clause(user_id: int, scope: str = "own"): async def owner_names_for(user_ids: set[int]) -> dict[int, str]: - """{user_id: username} in one query. Empty input costs nothing.""" + """{user_id: username} in one query. Empty input costs nothing. + + Fails soft: on a lookup error the caller gets no names and renders "another + user" instead. Losing an attribution is a cosmetic downgrade, and the part + that matters — that the record ISN'T the caller's — comes from comparing + owner ids, not from this. Failing the whole search over a username would be + the worse trade. + """ if not user_ids: return {} - async with async_session() as session: - rows = ( - await session.execute( - select(User.id, User.username).where(User.id.in_(user_ids)) - ) - ).all() - return {uid: name for uid, name in rows} + try: + async with async_session() as session: + rows = ( + await session.execute( + select(User.id, User.username).where(User.id.in_(user_ids)) + ) + ).all() + return {uid: name for uid, name in rows} + except Exception: + logger.warning("Owner-name lookup failed; falling back to unnamed " + "attribution", exc_info=True) + return {} def _my_group_ids(user_id: int): diff --git a/tests/test_mcp_tool_search.py b/tests/test_mcp_tool_search.py index 4b1f180..fa6de6a 100644 --- a/tests/test_mcp_tool_search.py +++ b/tests/test_mcp_tool_search.py @@ -18,13 +18,18 @@ def _reset_user_ctx(): def _fake_note(*, id: int, title: str, body: str = "", - tags: list[str] | None = None, is_task: bool = False) -> MagicMock: + tags: list[str] | None = None, is_task: bool = False, + user_id: int = 7) -> MagicMock: note = MagicMock() note.id = id note.title = title note.body = body note.tags = tags or [] note.is_task = is_task + # A real int, matching the bound caller by default: results compare it to + # decide whether to attach a shared/owner marker, and an auto-MagicMock would + # read as "someone else's" and send the tool looking up a username. + note.user_id = user_id return note diff --git a/tests/test_services_plugin_context.py b/tests/test_services_plugin_context.py index 0c30b4f..6281a47 100644 --- a/tests/test_services_plugin_context.py +++ b/tests/test_services_plugin_context.py @@ -10,9 +10,13 @@ def _rule(rid, title, topic_id): return r -def _note(nid, title): +def _note(nid, title, user_id=1): n = MagicMock() n.id, n.title = nid, title + # Real int, defaulting to the caller used in these tests: the injected menu + # compares it to decide whether the line needs a "shared by …" attribution, + # and an auto-MagicMock would read as another user's note. + n.user_id = user_id return n