8 Commits

Author SHA1 Message Date
e10331dd44 🐾 fix(api): workspace DELETE must not report success it didn't perform
DELETE returned 204 for a library owned by another user, having deleted
nothing. The caller (Daedalus) recorded the workspace as cleaned up and
dropped its own row, while the Library survived holding its globally-unique
name — so that name could never be reused, and nothing anywhere recorded why.

An unowned library now returns 409 owner_conflict, reusing the code the create
path already emits for the same condition. Ownership stays opaque on GET (404
as before): the disclosure concern is about reads, and a delete that silently
does nothing is the worse failure.

A genuinely absent library still returns 204 — that idempotency is relied upon
and is correct.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 13:03:32 -04:00
b5fba26d4c Merge pull request '🐾 fix(api): cascade collection and item deletes too' (#10) from fix/collection-item-cascade into main
All checks were successful
CVE Scan & Docker Build / security-scan (push) Successful in 4m24s
Build & Deploy Docs / build-and-deploy (push) Successful in 1m12s
CVE Scan & Docker Build / build-and-push (push) Successful in 2m38s
Reviewed-on: #10
2026-08-03 17:13:46 +00:00
15a517e389 🐾 fix(api): cascade collection and item deletes too
Collection and Item DELETE — both the REST endpoints and the HTML
views — called bare .delete(), leaking each collection's Items and every
item's Chunks/Images/ImageEmbeddings. New delete_collection_cascade and
delete_item_cascade in the shared library_delete service now back all
four paths. No orphan-Concept GC in either (that invariant stays
library-delete-only, matching the existing supersede path); the ingest
supersede helper in tasks.py now delegates to delete_item_cascade
instead of duplicating its Cypher.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-03 13:12:25 -04:00
c040e6f3dd Merge pull request '🐾 fix(api): cascade plain library delete; case-insensitive name conflict' (#9) from fix/library-api-delete-and-name-case into main
All checks were successful
CVE Scan & Docker Build / security-scan (push) Successful in 4m30s
Build & Deploy Docs / build-and-deploy (push) Successful in 1m16s
CVE Scan & Docker Build / build-and-push (push) Successful in 2m46s
Reviewed-on: #9
2026-08-03 16:45:48 +00:00
f1639dbdec 🐾 fix(api): cascade plain library delete; case-insensitive name conflict
DELETE /library/api/libraries/{uid}/ called bare lib.delete(), orphaning
Collections/Items/Chunks/Images and skipping Concept GC — the only delete
path not using the shared cascade. It now delegates to
delete_library_cascade like the HTML and workspace delete paths.

Name-conflict checks on both create endpoints are now case-insensitive
via find_library_by_name_ci (parameterised Cypher toLower comparison —
not neomodel iexact, which embeds the value in a regex and breaks on
names like "C++ Notes"). The Neo4j unique index is case-sensitive, so
"amazon connect" previously coexisted silently with "Amazon Connect",
confusing every name-matching client (Spelunker matches names
case-insensitively). The 409 reports the existing spelling. The plain
create also gains a UniqueProperty catch for the pre-check/save race,
mirroring the workspace path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-03 12:41:55 -04:00
7237468e3c Merge pull request '🐾 fix(tasks): unsupported file types fail ingest terminally, no retries' (#8) from fix/unsupported-file-type-terminal into main
All checks were successful
CVE Scan & Docker Build / security-scan (push) Successful in 4m11s
Build & Deploy Docs / build-and-deploy (push) Successful in 1m15s
CVE Scan & Docker Build / build-and-push (push) Successful in 2m49s
Reviewed-on: #8
2026-08-03 16:28:58 +00:00
e1f128659e 🐾 fix(tasks): unsupported file types fail ingest terminally, no retries
Unsupported file types raised a bare ValueError from the parser, which
ingest_from_daedalus treated like any transient fault — ERROR logs and
pointless Celery retries for input that can never parse. A new
UnsupportedFileTypeError (ValueError subclass, so existing callers still
catch it) is classified in the task as a terminal client-data failure:
logged at WARNING, job marked failed with reason
"unsupported_file_type", never retried.

Recovered from uncommitted work predating PR #6 (which deliberately
excluded it); test fixes on top: IngestJob's pk is id not job_id, and
.run() needs push_request() since the task persists self.request.id
into the NOT NULL celery_task_id column.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-03 12:27:24 -04:00
acf829e5ca Merge pull request '🐾 feat(library): per-app managed_by replaces hardcoded Daedalus badge' (#7) from feature/managed-by into main
All checks were successful
CVE Scan & Docker Build / security-scan (push) Successful in 4m34s
Build & Deploy Docs / build-and-deploy (push) Successful in 1m29s
CVE Scan & Docker Build / build-and-push (push) Successful in 2m42s
Reviewed-on: #7
2026-08-03 01:55:16 +00:00
12 changed files with 648 additions and 60 deletions

View File

@@ -85,26 +85,21 @@ an explicit `when: mnemosyne_first_deploy` flag.
```bash ```bash
# Apply Django ORM migrations (PostgreSQL schema) # Apply Django ORM migrations (PostgreSQL schema)
docker compose -f /srv/mnemosyne/docker-compose.yaml run --rm app migrate docker compose run --rm app migrate
# Create Neo4j vector + full-text indexes and load library-type defaults # Create Neo4j vector + full-text indexes and load library-type defaults
docker compose -f /srv/mnemosyne/docker-compose.yaml \ docker compose run --rm app setup
run --rm app setup
# Seed the MCPSigningKey used to sign long-lived Pallas team JWTs. # Seed the MCPSigningKey used to sign long-lived Pallas team JWTs.
# --retire-other deactivates any previously-active key. The hex # --retire-other deactivates any previously-active key. The hex
# emitted to stdout is persisted in Mnemosyne's database and is # emitted to stdout is persisted in Mnemosyne's database and is
# not re-injected from the vault — no operator action required # not re-injected from the vault — no operator action required
# beyond running this command once per fresh deployment. # beyond running this command once per fresh deployment.
docker compose -f /srv/mnemosyne/docker-compose.yaml \ docker compose run --rm app python manage.py seed_signing_key --kid daedalus-1 --retire-other
run --rm app \
python manage.py seed_signing_key --kid daedalus-1 --retire-other
# Create Django groups for SSO role mapping (View Only / Staff / SME / Admin). # Create Django groups for SSO role mapping (View Only / Staff / SME / Admin).
# Safe to re-run — idempotent. # Safe to re-run — idempotent.
docker compose -f /srv/mnemosyne/docker-compose.yaml \ docker compose run --rm app python manage.py create_sso_groups
run --rm app \
python manage.py create_sso_groups
``` ```
The `seed_signing_key` command prints the generated secret once to stdout — it The `seed_signing_key` command prints the generated secret once to stdout — it

View File

@@ -10,6 +10,7 @@ import os
from django.core.files.base import ContentFile from django.core.files.base import ContentFile
from django.core.files.storage import default_storage from django.core.files.storage import default_storage
from neomodel.exceptions import UniqueProperty
from rest_framework import status from rest_framework import status
from rest_framework.decorators import api_view, parser_classes, permission_classes from rest_framework.decorators import api_view, parser_classes, permission_classes
from rest_framework.parsers import FormParser, JSONParser, MultiPartParser from rest_framework.parsers import FormParser, JSONParser, MultiPartParser
@@ -17,6 +18,11 @@ from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response from rest_framework.response import Response
from library.content_types import get_library_type_config from library.content_types import get_library_type_config
from library.services.library_delete import (
delete_collection_cascade,
delete_item_cascade,
delete_library_cascade,
)
from mcp_server.drf_auth import request_token_label from mcp_server.drf_auth import request_token_label
from .serializers import ( from .serializers import (
@@ -51,7 +57,7 @@ def library_list_create(request):
a per-library ``item_count``. Off by default because the count is a a per-library ``item_count``. Off by default because the count is a
Cypher aggregate; on for the Daedalus-side registry poll. Cypher aggregate; on for the Daedalus-side registry poll.
""" """
from library.models import Library from library.models import Library, find_library_by_name_ci
if request.method == "GET": if request.method == "GET":
include_workspace = request.GET.get("include_workspace", "true").lower() != "false" include_workspace = request.GET.get("include_workspace", "true").lower() != "false"
@@ -85,13 +91,13 @@ def library_list_create(request):
serializer.is_valid(raise_exception=True) serializer.is_valid(raise_exception=True)
data = serializer.validated_data data = serializer.validated_data
# Library.name is globally unique; reject collisions with a clean 409 # Library names are unique. The Neo4j index is case-sensitive, but
# (uid + managed_by included so the caller can say who owns the name) # clients (Spelunker, humans) treat names case-insensitively, so the
# instead of letting the unique-index save raise a 500. # create-time check is case-insensitive too — otherwise "amazon connect"
try: # silently creates a near-duplicate of "Amazon Connect". Reject with a
existing = Library.nodes.get(name=data["name"]) # clean 409 (uid + managed_by included so the caller can say who owns
except Library.DoesNotExist: # the name) instead of letting the unique-index save raise a 500.
existing = None existing = find_library_by_name_ci(data["name"])
if existing is not None: if existing is not None:
logger.warning( logger.warning(
"library_create name_conflict name=%s existing_uid=%s caller=%s", "library_create name_conflict name=%s existing_uid=%s caller=%s",
@@ -99,7 +105,7 @@ def library_list_create(request):
) )
return Response( return Response(
{ {
"detail": f"A library named '{data['name']}' already exists.", "detail": f"A library named '{existing.name}' already exists.",
"code": "name_conflict", "code": "name_conflict",
"uid": existing.uid, "uid": existing.uid,
"managed_by": existing.managed_by_display or None, "managed_by": existing.managed_by_display or None,
@@ -127,7 +133,22 @@ def library_list_create(request):
data.get("llm_context_prompt") or defaults["llm_context_prompt"] data.get("llm_context_prompt") or defaults["llm_context_prompt"]
), ),
) )
lib.save() try:
lib.save()
except UniqueProperty:
# Race between the pre-check and save — an exact-name twin landed
# in between. Same 409 shape, minus the loser's uid/manager.
logger.warning(
"library_create name_conflict (save race) name=%s caller=%s",
data["name"], request.user.username,
)
return Response(
{
"detail": f"A library named '{data['name']}' already exists.",
"code": "name_conflict",
},
status=status.HTTP_409_CONFLICT,
)
return Response(LibrarySerializer(lib).data, status=status.HTTP_201_CREATED) return Response(LibrarySerializer(lib).data, status=status.HTTP_201_CREATED)
@@ -165,8 +186,16 @@ def library_detail(request, uid):
lib.save() lib.save()
return Response(LibrarySerializer(lib).data) return Response(LibrarySerializer(lib).data)
# DELETE # DELETE — use the shared cascade so child nodes (Collections/Items/
lib.delete() # Chunks/Images) and orphan Concepts are removed too; a bare
# lib.delete() would leak them all.
result = delete_library_cascade(lib)
logger.info(
"Library deleted via API library_uid=%s name=%s items=%d "
"orphans_deleted=%d caller=%s",
result["library_uid"], result["name"], result["item_count"],
result["orphans_deleted"], request.user.username,
)
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)
@@ -248,7 +277,13 @@ def collection_detail(request, uid):
col.save() col.save()
return Response(CollectionSerializer(col).data) return Response(CollectionSerializer(col).data)
col.delete() # DELETE — cascade Items/Chunks/Images too; a bare col.delete() leaks them.
result = delete_collection_cascade(col)
logger.info(
"Collection deleted via API collection_uid=%s name=%s items=%d caller=%s",
result["collection_uid"], result["name"], result["item_count"],
request.user.username,
)
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)
@@ -329,7 +364,13 @@ def item_detail(request, uid):
item.save() item.save()
return Response(ItemSerializer(item).data) return Response(ItemSerializer(item).data)
item.delete() # DELETE — cascade Chunks/Images/embeddings too; a bare item.delete()
# leaks them.
delete_item_cascade(item.uid)
logger.info(
"Item deleted via API item_uid=%s caller=%s",
item.uid, request.user.username,
)
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)

View File

@@ -67,7 +67,7 @@ def workspace_create(request):
workspace (200) — not an error. The library_type is frozen at first workspace (200) — not an error. The library_type is frozen at first
create; subsequent calls are not allowed to change it. create; subsequent calls are not allowed to change it.
""" """
from library.models import Library from library.models import Library, find_library_by_name_ci
serializer = WorkspaceCreateSerializer(data=request.data) serializer = WorkspaceCreateSerializer(data=request.data)
serializer.is_valid(raise_exception=True) serializer.is_valid(raise_exception=True)
@@ -127,6 +127,28 @@ def workspace_create(request):
status=status.HTTP_200_OK, status=status.HTTP_200_OK,
) )
# New workspace: reject a name already taken by any other library,
# case-insensitively — the Neo4j index is case-sensitive, so without
# this check "amazon connect" would silently coexist with an existing
# "Amazon Connect" and confuse every name-matching client.
name_taken = find_library_by_name_ci(data["name"])
if name_taken is not None:
logger.warning(
"workspace_create name_conflict workspace_id=%s name=%s "
"existing_uid=%s",
data["workspace_id"], data["name"], name_taken.uid,
)
return Response(
{
"detail": (
f"A library named '{name_taken.name}' already exists in "
"Mnemosyne."
),
"code": "name_conflict",
},
status=status.HTTP_409_CONFLICT,
)
defaults = get_library_type_config(data["library_type"]) defaults = get_library_type_config(data["library_type"])
lib = Library( lib = Library(
name=data["name"], name=data["name"],
@@ -191,23 +213,40 @@ def workspace_detail_or_delete(request, workspace_id):
except Library.DoesNotExist: except Library.DoesNotExist:
lib = None lib = None
# Cross-user reads/writes look like "not found" — don't disclose # Cross-user reads look like "not found" — don't disclose existence
# existence across users. # across users. DELETE is handled separately below: telling the caller
if lib is not None and lib.owner_username != request.user.username: # "deleted" about a library we did not touch is what silently orphans
lib = None # libraries, and an orphan holds its globally-unique name forever.
unowned = lib is not None and lib.owner_username != request.user.username
if request.method == "GET": if request.method == "GET":
if lib is None: if lib is None or unowned:
return Response( return Response(
{"detail": "Workspace not found."}, {"detail": "Workspace not found."},
status=status.HTTP_404_NOT_FOUND, status=status.HTTP_404_NOT_FOUND,
) )
return Response(WorkspaceStatusSerializer(_serialize_workspace(lib)).data) 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: if lib is None:
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)
if unowned:
# 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 # Delete the Library and everything reachable + unique to it, plus
# orphan-Concept GC. Shared with the admin/HTML delete path. # orphan-Concept GC. Shared with the admin/HTML delete path.
result = delete_library_cascade(lib) result = delete_library_cascade(lib)

View File

@@ -63,6 +63,23 @@ def infer_legacy_manager(workspace_id):
return "Kairos" if workspace_id.startswith("kairos-mail-") else "Daedalus" return "Kairos" if workspace_id.startswith("kairos-mail-") else "Daedalus"
def find_library_by_name_ci(name):
"""Case-insensitively find a Library by name, or None.
Parameterised Cypher rather than neomodel's ``iexact``, which embeds
the value in a regex and so breaks on names containing regex
metacharacters (e.g. "C++ Notes").
"""
from neomodel import db
rows, _ = db.cypher_query(
"MATCH (l:Library) WHERE toLower(l.name) = toLower($name) "
"RETURN l LIMIT 1",
{"name": name},
)
return Library.inflate(rows[0][0]) if rows else None
class Library(StructuredNode): class Library(StructuredNode):
""" """
Top-level container representing a content library. Top-level container representing a content library.

View File

@@ -106,3 +106,99 @@ def delete_library_cascade(lib) -> dict:
"item_s3_keys": item_s3_keys, "item_s3_keys": item_s3_keys,
"orphans_deleted": orphans_deleted, "orphans_deleted": orphans_deleted,
} }
def delete_collection_cascade(col) -> dict:
"""Delete ``col`` and all content reachable and unique to it.
Removes the Collection's Items with their Chunks, Images, and
ImageEmbeddings, then the Collection itself. No orphan-Concept GC —
that invariant belongs to library-level deletes only (see
:func:`delete_library_cascade`); Concepts orphaned here are collected
on the next library delete.
:param col: A ``library.models.Collection`` node instance.
:returns: Dict with ``collection_uid``, ``name``, ``item_count``, and
``item_s3_keys`` (list of ``(uid, s3_key)`` for async S3 cleanup).
"""
collection_uid = col.uid
collection_name = col.name
s3_rows, _ = db.cypher_query(
"MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item) "
"RETURN i.uid, i.s3_key",
{"uid": collection_uid},
)
item_s3_keys = [(r[0], r[1]) for r in s3_rows if r[1]]
db.cypher_query(
"""
MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item)
-[:HAS_CHUNK]->(c:Chunk)
DETACH DELETE c
""",
{"uid": collection_uid},
)
db.cypher_query(
"""
MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item)
-[:HAS_IMAGE]->(img:Image)
OPTIONAL MATCH (img)-[:HAS_EMBEDDING]->(emb:ImageEmbedding)
DETACH DELETE img, emb
""",
{"uid": collection_uid},
)
db.cypher_query(
"""
MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item)
DETACH DELETE i
""",
{"uid": collection_uid},
)
db.cypher_query(
"MATCH (col:Collection {uid: $uid}) DETACH DELETE col",
{"uid": collection_uid},
)
logger.info(
"Collection cascade-deleted collection_uid=%s name=%s items=%d",
collection_uid, collection_name, len(item_s3_keys),
)
return {
"collection_uid": collection_uid,
"name": collection_name,
"item_count": len(item_s3_keys),
"item_s3_keys": item_s3_keys,
}
def delete_item_cascade(item_uid: str) -> dict:
"""Delete the Item ``item_uid`` with its Chunks, Images, and embeddings.
Keyed on the uid (not a node instance) so the ingest supersede path in
``library.tasks`` can share it. No orphan-Concept GC — see
:func:`delete_collection_cascade`.
:returns: Dict with ``item_uid`` and ``s3_key`` (empty string when the
item had no stored file) for async S3 cleanup.
"""
s3_rows, _ = db.cypher_query(
"MATCH (i:Item {uid: $uid}) RETURN i.s3_key",
{"uid": item_uid},
)
s3_key = (s3_rows[0][0] or "") if s3_rows else ""
db.cypher_query(
"""
MATCH (i:Item {uid: $uid})
OPTIONAL MATCH (i)-[:HAS_CHUNK]->(c:Chunk)
OPTIONAL MATCH (i)-[:HAS_IMAGE]->(img:Image)
OPTIONAL MATCH (img)-[:HAS_EMBEDDING]->(emb:ImageEmbedding)
DETACH DELETE c, img, emb, i
""",
{"uid": item_uid},
)
logger.info("Item cascade-deleted item_uid=%s", item_uid)
return {"item_uid": item_uid, "s3_key": s3_key}

View File

@@ -22,6 +22,17 @@ from .text_utils import remove_excessive_whitespace, sanitize_text
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class UnsupportedFileTypeError(ValueError):
"""Raised when a file's type cannot be parsed.
Deterministic and input-driven — re-parsing identical bytes can never
succeed — so ingest must treat it as a terminal failure and never retry.
Subclasses ``ValueError`` so existing ``except ValueError`` callers still
catch it.
"""
# File extensions supported by PyMuPDF # File extensions supported by PyMuPDF
PYMUPDF_EXTENSIONS = { PYMUPDF_EXTENSIONS = {
"pdf", "epub", "xps", "mobi", "fb2", "cbz", "svg", "pdf", "epub", "xps", "mobi", "fb2", "cbz", "svg",
@@ -88,7 +99,7 @@ class DocumentParser:
:param file_path: Path to the document file. :param file_path: Path to the document file.
:param file_type: File extension (without dot), e.g. 'pdf', 'epub'. :param file_type: File extension (without dot), e.g. 'pdf', 'epub'.
:returns: ParseResult with text blocks, images, and metadata. :returns: ParseResult with text blocks, images, and metadata.
:raises ValueError: If the file type is not supported. :raises UnsupportedFileTypeError: If the file type is not supported.
""" """
file_type = file_type.lower().lstrip(".") file_type = file_type.lower().lstrip(".")
@@ -117,7 +128,7 @@ class DocumentParser:
if file_type in ("html", "htm"): if file_type in ("html", "htm"):
return self._parse_with_pymupdf(file_path, file_type) return self._parse_with_pymupdf(file_path, file_type)
raise ValueError( raise UnsupportedFileTypeError(
f"Unsupported file type '{file_type}'. " f"Unsupported file type '{file_type}'. "
f"Supported: {sorted(PYMUPDF_EXTENSIONS | PLAINTEXT_EXTENSIONS | IMAGE_EXTENSIONS)}" f"Supported: {sorted(PYMUPDF_EXTENSIONS | PLAINTEXT_EXTENSIONS | IMAGE_EXTENSIONS)}"
) )

View File

@@ -347,6 +347,7 @@ def ingest_from_daedalus(self, job_id: str):
from datetime import datetime, timezone from datetime import datetime, timezone
from library.models import IngestJob, Item, Library from library.models import IngestJob, Item, Library
from library.services.parsers import UnsupportedFileTypeError
from library.services.source_s3 import ( from library.services.source_s3 import (
copy_into_mnemosyne, copy_into_mnemosyne,
fetch_from_source, fetch_from_source,
@@ -465,6 +466,24 @@ def ingest_from_daedalus(self, job_id: str):
**result, **result,
} }
except UnsupportedFileTypeError as exc:
# Deterministic, input-driven — re-parsing identical bytes can never
# succeed. Terminal client-data failure, never retried, and logged at
# WARNING (not ERROR) because an unparseable input is not a server fault.
logger.warning(
"Task ingest_from_daedalus rejected job_id=%s: %s", job_id, exc,
)
job.status = "failed"
job.error = str(exc)
job.completed_at = datetime.now(timezone.utc)
job.save(update_fields=["status", "error", "completed_at"])
return {
"success": False,
"job_id": job_id,
"error": str(exc),
"reason": "unsupported_file_type",
}
except Exception as exc: except Exception as exc:
logger.error( logger.error(
"Task ingest_from_daedalus failed job_id=%s: %s", "Task ingest_from_daedalus failed job_id=%s: %s",
@@ -484,16 +503,9 @@ def ingest_from_daedalus(self, job_id: str):
def _delete_item_and_chunks(item_uid: str): def _delete_item_and_chunks(item_uid: str):
"""Delete an Item, its chunks, and its images. Concept GC is workspace-delete only.""" """Delete an Item, its chunks, and its images. Concept GC is workspace-delete only."""
db.cypher_query( from library.services.library_delete import delete_item_cascade
"""
MATCH (i:Item {uid: $uid}) delete_item_cascade(item_uid)
OPTIONAL MATCH (i)-[:HAS_CHUNK]->(c:Chunk)
OPTIONAL MATCH (i)-[:HAS_IMAGE]->(img:Image)
OPTIONAL MATCH (img)-[:HAS_EMBEDDING]->(emb:ImageEmbedding)
DETACH DELETE c, img, emb, i
""",
{"uid": item_uid},
)
def _resolve_or_create_default_collection(lib, collection_uid: str = ""): def _resolve_or_create_default_collection(lib, collection_uid: str = ""):

View File

@@ -0,0 +1,148 @@
"""Tests for the plain library REST endpoints beyond create.
Currently covers the DELETE cascades: library, collection, and item DELETE
endpoints must go through the shared ``library_delete`` service functions —
bare ``.delete()`` calls leak child nodes (Collections, Items, Chunks,
Images) and, for libraries, skip orphan-Concept GC. Neo4j is stubbed via
``sys.modules``, same style as ``test_managed_by.py``.
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
from django.contrib.auth import get_user_model
from django.test import TestCase
from rest_framework.test import APIClient
User = get_user_model()
def _fake_node_cls(instance):
"""A neomodel-class stand-in whose ``nodes.get`` returns ``instance``.
``instance=None`` makes ``nodes.get`` raise the class's DoesNotExist.
"""
fake_nodes = MagicMock()
class DoesNotExist(Exception):
pass
if instance is None:
fake_nodes.get.side_effect = DoesNotExist()
else:
fake_nodes.get.return_value = instance
return SimpleNamespace(nodes=fake_nodes, DoesNotExist=DoesNotExist)
class LibraryApiDeleteTests(TestCase):
"""DELETE on the plain library endpoint cascades."""
def setUp(self):
self.user = User.objects.create_user(username="op", password="pw")
self.client = APIClient()
self.client.force_authenticate(user=self.user)
def _fake_models_module(self, lib):
return SimpleNamespace(Library=_fake_node_cls(lib))
def test_delete_uses_shared_cascade(self):
lib = SimpleNamespace(uid="lib-1", name="Docs")
cascade_result = {
"library_uid": "lib-1",
"name": "Docs",
"item_count": 3,
"item_s3_keys": [],
"orphans_deleted": 1,
}
with patch.dict(
"sys.modules", {"library.models": self._fake_models_module(lib)}
), patch(
"library.api.views.delete_library_cascade",
return_value=cascade_result,
) as mock_cascade:
response = self.client.delete("/library/api/libraries/lib-1/")
self.assertEqual(response.status_code, 204)
mock_cascade.assert_called_once_with(lib)
def test_delete_missing_library_returns_404(self):
with patch.dict(
"sys.modules", {"library.models": self._fake_models_module(None)}
), patch("library.api.views.delete_library_cascade") as mock_cascade:
response = self.client.delete("/library/api/libraries/nope/")
self.assertEqual(response.status_code, 404)
mock_cascade.assert_not_called()
class CollectionApiDeleteTests(TestCase):
"""DELETE on the collection endpoint cascades Items/Chunks/Images."""
def setUp(self):
self.user = User.objects.create_user(username="op", password="pw")
self.client = APIClient()
self.client.force_authenticate(user=self.user)
def test_delete_uses_shared_cascade(self):
col = SimpleNamespace(uid="col-1", name="Default")
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Collection=_fake_node_cls(col))},
), patch(
"library.api.views.delete_collection_cascade",
return_value={
"collection_uid": "col-1",
"name": "Default",
"item_count": 2,
"item_s3_keys": [],
},
) as mock_cascade:
response = self.client.delete("/library/api/collections/col-1/")
self.assertEqual(response.status_code, 204)
mock_cascade.assert_called_once_with(col)
def test_delete_missing_collection_returns_404(self):
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Collection=_fake_node_cls(None))},
), patch("library.api.views.delete_collection_cascade") as mock_cascade:
response = self.client.delete("/library/api/collections/nope/")
self.assertEqual(response.status_code, 404)
mock_cascade.assert_not_called()
class ItemApiDeleteTests(TestCase):
"""DELETE on the item endpoint cascades Chunks/Images/embeddings."""
def setUp(self):
self.user = User.objects.create_user(username="op", password="pw")
self.client = APIClient()
self.client.force_authenticate(user=self.user)
def test_delete_uses_shared_cascade(self):
item = SimpleNamespace(uid="item-1", title="Doc")
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Item=_fake_node_cls(item))},
), patch(
"library.api.views.delete_item_cascade",
return_value={"item_uid": "item-1", "s3_key": ""},
) as mock_cascade:
response = self.client.delete("/library/api/items/item-1/")
self.assertEqual(response.status_code, 204)
mock_cascade.assert_called_once_with("item-1")
def test_delete_missing_item_returns_404(self):
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Item=_fake_node_cls(None))},
), patch("library.api.views.delete_item_cascade") as mock_cascade:
response = self.client.delete("/library/api/items/nope/")
self.assertEqual(response.status_code, 404)
mock_cascade.assert_not_called()

View File

@@ -70,8 +70,9 @@ class _FakeLibrary:
class DoesNotExist(Exception): class DoesNotExist(Exception):
pass pass
existing = None # what nodes.get(name=...) returns existing = None # what find_library_by_name_ci returns
instances = [] # constructor kwargs, in order instances = [] # constructor kwargs, in order
save_raises = None # exception instance save() should raise, if any
def __init__(self, **kwargs): def __init__(self, **kwargs):
type(self).instances.append(kwargs) type(self).instances.append(kwargs)
@@ -81,16 +82,20 @@ class _FakeLibrary:
self.created_at = None self.created_at = None
def save(self): def save(self):
if type(self).save_raises is not None:
raise type(self).save_raises
return self return self
class _Nodes:
@staticmethod
def get(**kwargs):
if _FakeLibrary.existing is None:
raise _FakeLibrary.DoesNotExist()
return _FakeLibrary.existing
nodes = _Nodes() def _fake_find_ci(name):
_FakeLibrary.ci_queries.append(name)
return _FakeLibrary.existing
def _fake_models_module():
return SimpleNamespace(
Library=_FakeLibrary, find_library_by_name_ci=_fake_find_ci
)
class LibraryCreateStampingTests(TestCase): class LibraryCreateStampingTests(TestCase):
@@ -101,16 +106,15 @@ class LibraryCreateStampingTests(TestCase):
self.client = APIClient() self.client = APIClient()
_FakeLibrary.existing = None _FakeLibrary.existing = None
_FakeLibrary.instances = [] _FakeLibrary.instances = []
_FakeLibrary.save_raises = None
_FakeLibrary.ci_queries = []
def _post(self, token=None): def _post(self, token=None, name="Docs"):
self.client.force_authenticate(user=self.user, token=token) self.client.force_authenticate(user=self.user, token=token)
with patch.dict( with patch.dict("sys.modules", {"library.models": _fake_models_module()}):
"sys.modules",
{"library.models": SimpleNamespace(Library=_FakeLibrary)},
):
return self.client.post( return self.client.post(
"/library/api/libraries/", "/library/api/libraries/",
{"name": "Docs", "library_type": "technical"}, {"name": name, "library_type": "technical"},
format="json", format="json",
) )
@@ -129,7 +133,7 @@ class LibraryCreateStampingTests(TestCase):
def test_duplicate_name_returns_409_name_conflict(self): def test_duplicate_name_returns_409_name_conflict(self):
_FakeLibrary.existing = SimpleNamespace( _FakeLibrary.existing = SimpleNamespace(
uid="lib-old", managed_by_display="Daedalus" uid="lib-old", name="Docs", managed_by_display="Daedalus"
) )
response = self._post(token=UserToken(name="Spelunker")) response = self._post(token=UserToken(name="Spelunker"))
@@ -141,15 +145,39 @@ class LibraryCreateStampingTests(TestCase):
self.assertEqual(body["managed_by"], "Daedalus") self.assertEqual(body["managed_by"], "Daedalus")
self.assertEqual(_FakeLibrary.instances, []) self.assertEqual(_FakeLibrary.instances, [])
def test_duplicate_check_is_case_insensitive(self):
"""A case-variant name 409s and reports the existing spelling."""
_FakeLibrary.existing = SimpleNamespace(
uid="lib-old", name="Amazon Connect", managed_by_display="Spelunker"
)
response = self._post(token=None, name="amazon connect")
self.assertEqual(response.status_code, 409)
# The lookup received the posted name (case-folding happens in
# Cypher), and the response names the existing spelling.
self.assertEqual(_FakeLibrary.ci_queries, ["amazon connect"])
self.assertIn("Amazon Connect", response.json()["detail"])
self.assertEqual(_FakeLibrary.instances, [])
def test_duplicate_of_unmanaged_reports_null_manager(self): def test_duplicate_of_unmanaged_reports_null_manager(self):
_FakeLibrary.existing = SimpleNamespace( _FakeLibrary.existing = SimpleNamespace(
uid="lib-old", managed_by_display="" uid="lib-old", name="Docs", managed_by_display=""
) )
response = self._post(token=None) response = self._post(token=None)
self.assertEqual(response.status_code, 409) self.assertEqual(response.status_code, 409)
self.assertIsNone(response.json()["managed_by"]) self.assertIsNone(response.json()["managed_by"])
def test_save_race_returns_409_not_500(self):
"""An exact-name twin landing between pre-check and save 409s."""
from neomodel.exceptions import UniqueProperty
_FakeLibrary.save_raises = UniqueProperty("name")
response = self._post(token=None)
self.assertEqual(response.status_code, 409)
self.assertEqual(response.json()["code"], "name_conflict")
class WorkspaceStatusSerializerManagedByTests(TestCase): class WorkspaceStatusSerializerManagedByTests(TestCase):
"""The workspace status payload carries ``managed_by`` (nullable).""" """The workspace status payload carries ``managed_by`` (nullable)."""

View File

@@ -67,6 +67,114 @@ class ReembedItemTaskTests(TestCase):
mock_pipeline.reprocess_item.assert_called_once() mock_pipeline.reprocess_item.assert_called_once()
@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
class IngestFromDaedalusFailureClassificationTests(TestCase):
"""A deterministic parse failure is terminal; a transient error retries.
Neo4j and S3 are mocked at the task's boundaries so the test exercises the
exception-classification branch (parsers.UnsupportedFileTypeError vs. any
other Exception) without a live graph or object store.
"""
def setUp(self):
from library.tasks import ingest_from_daedalus
# Calling .run() bypasses Celery's request setup, but the task
# persists self.request.id into the NOT NULL celery_task_id column
# and reads self.request.retries — push a real request context.
ingest_from_daedalus.push_request(id="test-task-id", retries=0)
self.addCleanup(ingest_from_daedalus.pop_request)
def _make_job(self):
from library.models import IngestJob
return IngestJob.objects.create(
id="job_test_unsupported",
library_uid="lib-uid-123",
source="daedalus",
s3_key="incoming/bad.drawio",
file_type="vnd.jgraph.mxfile",
title="bad.drawio",
content_hash="abc123",
)
def _patched_boundaries(self, pipeline_side_effect):
"""Patch every boundary the task hits before the pipeline runs.
Returns a context-manager list; the pipeline's ``process_item`` is set
to raise ``pipeline_side_effect``.
"""
from library.services.parsers import UnsupportedFileTypeError # noqa: F401
patchers = [
patch("library.tasks.db"),
patch("library.models.Library"),
patch("library.models.Item"),
patch("library.services.source_s3.fetch_from_source", return_value=b"data"),
patch("library.services.source_s3.copy_into_mnemosyne"),
patch("library.tasks._resolve_or_create_default_collection"),
patch("library.services.pipeline.EmbeddingPipeline"),
]
mocks = [p.start() for p in patchers]
self.addCleanup(lambda: [p.stop() for p in patchers])
# db.cypher_query returns (rows, meta); no prior item to supersede.
mocks[0].cypher_query.return_value = ([], None)
# Library.nodes.get returns a stand-in library node.
mocks[1].nodes.get.return_value = MagicMock(uid="lib-uid-123")
# Item() instances carry a uid used for the S3 key.
item_instance = MagicMock(uid="item-uid-abc")
mocks[2].return_value = item_instance
# The pipeline raises the classification-relevant error.
pipeline_instance = MagicMock()
pipeline_instance.process_item.side_effect = pipeline_side_effect
mocks[6].return_value = pipeline_instance
return pipeline_instance
def test_unsupported_file_type_is_terminal_and_not_retried(self):
from library.models import IngestJob
from library.services.parsers import UnsupportedFileTypeError
from library.tasks import ingest_from_daedalus
job = self._make_job()
self._patched_boundaries(
UnsupportedFileTypeError("Unsupported file type 'vnd.jgraph.mxfile'.")
)
with patch.object(ingest_from_daedalus, "retry") as mock_retry:
result = ingest_from_daedalus.run(job.id)
# Never retried.
mock_retry.assert_not_called()
# Terminal failure with a machine-readable reason.
self.assertFalse(result["success"])
self.assertEqual(result["reason"], "unsupported_file_type")
job.refresh_from_db()
self.assertEqual(job.status, "failed")
self.assertEqual(job.retry_count, 0)
self.assertIsNotNone(job.completed_at)
self.assertIn("Unsupported file type", job.error)
def test_transient_error_takes_the_retry_path(self):
from library.tasks import ingest_from_daedalus
job = self._make_job()
self._patched_boundaries(ConnectionError("S3 hiccup"))
# self.retry raises Retry in real Celery; simulate that so the task
# doesn't fall through to the terminal branch.
from celery.exceptions import Retry
with patch.object(ingest_from_daedalus, "retry", side_effect=Retry()) as mock_retry:
with self.assertRaises(Retry):
ingest_from_daedalus.run(job.id)
mock_retry.assert_called_once()
job.refresh_from_db()
self.assertEqual(job.retry_count, 1)
class ResolveUserTests(TestCase): class ResolveUserTests(TestCase):
"""Tests for the _resolve_user helper.""" """Tests for the _resolve_user helper."""

View File

@@ -7,6 +7,9 @@ search scoping) require Neo4j and are validated by the manual end-to-end
test plan, not these unit tests. 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 django.test import TestCase
from rest_framework.test import APIClient from rest_framework.test import APIClient
@@ -208,3 +211,85 @@ class WorkspaceEndpointAuthTests(TestCase):
def test_workspace_delete_requires_auth(self): def test_workspace_delete_requires_auth(self):
response = self.client.delete("/library/api/workspaces/ws_a/") response = self.client.delete("/library/api/workspaces/ws_a/")
self.assertIn(response.status_code, [401, 403]) 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_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)

View File

@@ -460,7 +460,11 @@ def collection_delete(request, uid):
if request.method == "POST": if request.method == "POST":
name = col.name name = col.name
col.delete() # Shared cascade so Items/Chunks/Images go too — a bare
# col.delete() would leak them.
from .services.library_delete import delete_collection_cascade
delete_collection_cascade(col)
messages.success(request, f'Collection "{name}" deleted.') messages.success(request, f'Collection "{name}" deleted.')
return redirect("library:library-list") return redirect("library:library-list")
return render( return render(
@@ -648,7 +652,11 @@ def item_delete(request, uid):
if request.method == "POST": if request.method == "POST":
title = item.title title = item.title
item.delete() # Shared cascade so Chunks/Images/embeddings go too — a bare
# item.delete() would leak them.
from .services.library_delete import delete_item_cascade
delete_item_cascade(item.uid)
messages.success(request, f'Item "{title}" deleted.') messages.success(request, f'Item "{title}" deleted.')
return redirect("library:library-list") return redirect("library:library-list")
return render(request, "library/item_confirm_delete.html", {"item": item}) return render(request, "library/item_confirm_delete.html", {"item": item})