From e9cd3435ad06025e9bb8cf63db519fc075de3f1b Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Sat, 25 Jul 2026 23:10:12 -0400
Subject: [PATCH] test(acl): give search/inject fixtures real owner ids; fail
 soft on name lookup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Run 2892: 3 pre-existing tests broke, 392 passed. Same shape as the last
breakage — a DB-touching call landed in a path unit tests exercise, and their
fixtures had auto-MagicMock user_id attributes that compare as "someone else's",
sending the code off to look up a username.

Fixed the fixtures rather than the assertion: _fake_note in the search tests and
_note in the plugin-context tests now take a real user_id defaulting to the
caller those tests bind. That makes "is this shared?" meaningful in both files
instead of accidental, which is what the new provenance behaviour actually needs
from them.

Separately, and not as cover for the above: owner_names_for now fails soft.
A lookup error yields no names and callers render "another user". The part that
matters — that the record is NOT the caller's — comes from comparing owner ids,
not from this query, so losing an attribution is cosmetic where failing the whole
search would not be.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt
---
 src/scribe/services/access.py         | 28 +++++++++++++++++++--------
 tests/test_mcp_tool_search.py         |  7 ++++++-
 tests/test_services_plugin_context.py |  6 +++++-
 3 files changed, 31 insertions(+), 10 deletions(-)

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