🐾 fix(api): treat a NULL owner as unclaimed, not foreign

The previous commit's guard rejected any library whose owner_username didn't
match the caller — including NULL. On the live instance 12 of 16 workspace
libraries have no owner (created before owner stamping, or via the plain
/libraries/ endpoint, which never sets one), so that turned a silent false
204 into a hard 409 and made them undeletable. Strictly worse: the orphan
became permanent rather than merely unrecorded.

A NULL owner is unclaimed. Only a *different* non-null owner is foreign, and
only that returns 409.

Verified against live Neo4j on umbriel with throwaway libraries: NULL owner
deletes (204, node gone), caller-owned deletes (204, node gone), other-owned
is refused (409, node survives), absent stays 204. Test data cleaned up; the
30 real libraries were untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

mnemosyne/library/api/workspaces.py

@@ -213,23 +213,50 @@ def workspace_detail_or_delete(request, workspace_id):
     except Library.DoesNotExist:
         lib = None
 
-    # Cross-user reads/writes look like "not found" — don't disclose
-    # existence across users.
-    if lib is not None and lib.owner_username != request.user.username:
-        lib = None
+    # A NULL owner_username is *unclaimed*, not someone else's: workspace
+    # libraries created before owner stamping (and any created through the
+    # plain /libraries/ endpoint, which never sets it) have no owner. Treating
+    # those as foreign would make them permanently undeletable through this
+    # endpoint — the very orphans this view is trying not to create.
+    #
+    # Cross-user reads still look like "not found"; DELETE is handled
+    # separately below, because telling the caller "deleted" about a library
+    # we did not touch is what silently orphans libraries, and an orphan holds
+    # its globally-unique name forever.
+    foreign = (
+        lib is not None
+        and lib.owner_username
+        and lib.owner_username != request.user.username
+    )
 
     if request.method == "GET":
-        if lib is None:
+        if lib is None or foreign:
             return Response(
                 {"detail": "Workspace not found."},
                 status=status.HTTP_404_NOT_FOUND,
             )
         return Response(WorkspaceStatusSerializer(_serialize_workspace(lib)).data)
 
-    # DELETE — idempotent: a missing (or unowned) workspace returns 204.
+    # DELETE — idempotent only where nothing exists to delete.
     if lib is None:
         return Response(status=status.HTTP_204_NO_CONTENT)
 
+    if foreign:
+        # Still opaque about ownership, but never a false success: the
+        # caller must not record this workspace as cleaned up.
+        logger.warning(
+            "workspace_delete owner_conflict workspace_id=%s library_uid=%s "
+            "caller=%s",
+            workspace_id, lib.uid, request.user.username,
+        )
+        return Response(
+            {
+                "detail": "Workspace id is already in use.",
+                "code": "owner_conflict",
+            },
+            status=status.HTTP_409_CONFLICT,
+        )
+
     # Delete the Library and everything reachable + unique to it, plus
     # orphan-Concept GC. Shared with the admin/HTML delete path.
     result = delete_library_cascade(lib)

mnemosyne/library/tests/test_workspaces_api.py

@@ -7,6 +7,9 @@ search scoping) require Neo4j and are validated by the manual end-to-end
 test plan, not these unit tests.
 """
 
+from unittest.mock import Mock, patch
+
+from django.contrib.auth.models import User
 from django.test import TestCase
 from rest_framework.test import APIClient
 
@@ -208,3 +211,113 @@ class WorkspaceEndpointAuthTests(TestCase):
     def test_workspace_delete_requires_auth(self):
         response = self.client.delete("/library/api/workspaces/ws_a/")
         self.assertIn(response.status_code, [401, 403])
+
+
+class WorkspaceDeleteOwnershipTests(TestCase):
+    """DELETE must never report success for a library it did not delete.
+
+    A false 204 is how libraries get orphaned: Daedalus records the
+    workspace as cleaned up and drops its own row, while the Library node
+    survives holding its globally-unique name forever — which then blocks
+    ever recreating a workspace under that name.
+    """
+
+    def setUp(self):
+        self.client = APIClient()
+        self.user = User.objects.create_user(
+            username="owner", password="pw"  # noqa: S106 — test credential
+        )
+        self.client.force_authenticate(user=self.user)
+
+    def _library(self, owner_username):
+        lib = Mock()
+        lib.uid = "lib_1"
+        lib.owner_username = owner_username
+        return lib
+
+    def test_absent_library_still_returns_204(self):
+        """Genuine idempotency is preserved — nothing exists, nothing to do."""
+        from library.models import Library
+
+        with patch(
+            "neomodel.sync_.match.NodeSet.get",
+            side_effect=Library.DoesNotExist("nope"),
+        ):
+            response = self.client.delete("/library/api/workspaces/ws_gone/")
+
+        self.assertEqual(response.status_code, 204)
+
+    def test_unowned_library_returns_409_and_is_not_deleted(self):
+        from library.models import Library
+
+        lib = self._library("someone_else")
+        with (
+            patch("neomodel.sync_.match.NodeSet.get", return_value=lib),
+            patch(
+                "library.api.workspaces.delete_library_cascade"
+            ) as cascade,
+        ):
+            response = self.client.delete("/library/api/workspaces/ws_a/")
+
+        self.assertEqual(response.status_code, 409)
+        self.assertEqual(response.json()["code"], "owner_conflict")
+        cascade.assert_not_called()
+
+    def test_owned_library_is_deleted(self):
+        from library.models import Library
+
+        lib = self._library("owner")
+        with (
+            patch("neomodel.sync_.match.NodeSet.get", return_value=lib),
+            patch(
+                "library.api.workspaces.delete_library_cascade",
+                return_value={
+                    "library_uid": "lib_1",
+                    "name": "Assistant",
+                    "item_count": 0,
+                    "orphans_deleted": 0,
+                },
+            ) as cascade,
+        ):
+            response = self.client.delete("/library/api/workspaces/ws_a/")
+
+        self.assertEqual(response.status_code, 204)
+        cascade.assert_called_once_with(lib)
+
+    def test_unclaimed_library_is_deleted(self):
+        """A NULL owner is unclaimed, not foreign.
+
+        Workspace libraries predating owner stamping (and any created via the
+        plain /libraries/ endpoint, which never sets an owner) have
+        owner_username=None. Refusing those would make them permanently
+        undeletable through this endpoint — orphans by construction.
+        """
+        from library.models import Library
+
+        lib = self._library(None)
+        with (
+            patch("neomodel.sync_.match.NodeSet.get", return_value=lib),
+            patch(
+                "library.api.workspaces.delete_library_cascade",
+                return_value={
+                    "library_uid": "lib_1",
+                    "name": "Assistant",
+                    "item_count": 0,
+                    "orphans_deleted": 0,
+                },
+            ) as cascade,
+        ):
+            response = self.client.delete("/library/api/workspaces/ws_a/")
+
+        self.assertEqual(response.status_code, 204)
+        cascade.assert_called_once_with(lib)
+
+    def test_unowned_library_get_still_looks_absent(self):
+        """Ownership must stay opaque on reads — 404, not 409."""
+        from library.models import Library
+
+        lib = self._library("someone_else")
+        with patch("neomodel.sync_.match.NodeSet.get", return_value=lib):
+            response = self.client.get("/library/api/workspaces/ws_a/")
+
+        self.assertEqual(response.status_code, 404)