diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 5e3ce3f6..9ab445a6 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -7,7 +7,7 @@ on: branches: [ main, master ] jobs: - test: + test-and-typecheck: runs-on: ubuntu-latest strategy: matrix: @@ -15,15 +15,17 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Install dependencies run: | python -m pip install --upgrade pip pip install -r requirements.txt - pip install pytest - - name: Run tests + pip install pytest mypy + - name: Run pytest run: | - python -m unittest discover -v - pytest -q || true + python -m pytest -q + - name: Run mypy (strict) + run: | + python -m mypy --config-file mypy.ini diff --git a/README.md b/README.md index 170598f0..05115085 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,12 @@ # Majoor Assets Manager for ComfyUI +[![GitHub Repo](https://img.shields.io/badge/GitHub-Repo-181717?logo=github)](https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager) +[![GitHub Stars](https://img.shields.io/github/stars/MajoorWaldi/ComfyUI-Majoor-AssetsManager?style=flat)](https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager/stargazers) +[![GitHub Forks](https://img.shields.io/github/forks/MajoorWaldi/ComfyUI-Majoor-AssetsManager?style=flat)](https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager/network/members) +[![GitHub Issues](https://img.shields.io/github/issues/MajoorWaldi/ComfyUI-Majoor-AssetsManager?style=flat)](https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager/issues) +[![License](https://img.shields.io/github/license/MajoorWaldi/ComfyUI-Majoor-AssetsManager?style=flat)](LICENSE) +[![CI](https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager/actions/workflows/python-tests.yml/badge.svg)](https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager/actions/workflows/python-tests.yml) + Majoor Assets Manager is an advanced asset browser for ComfyUI that provides a comprehensive solution for managing, organizing, and viewing your generated assets. It integrates directly into ComfyUI, offering features like full-text search, metadata extraction, rating and tagging systems, and advanced viewing capabilities. ![Majoor Assets Manager Demo](examples/ComfyUI_Majoor_AssetsManager_Video.gif) @@ -7,6 +14,11 @@ Majoor Assets Manager is an advanced asset browser for ComfyUI that provides a c **[User Guide](user_guide.html)** - Complete documentation with features, installation, and usage instructions. **[Documentation Index](docs/DOCUMENTATION_INDEX.md)** - All docs (install, viewer, metadata, settings, etc). +## GitHub + +- **Repository:** https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager +- **Author:** MajoorWaldi + ## License Copyright (c) 2026 Ewald ALOEBOETOE (MajoorWaldi). diff --git a/backend/adapters/db/schema.py b/backend/adapters/db/schema.py index 609163e3..b97a26c1 100644 --- a/backend/adapters/db/schema.py +++ b/backend/adapters/db/schema.py @@ -201,6 +201,7 @@ def _get_table_columns(db, table_name: str) -> Result[List[str]]: def table_has_column(db, table_name: str, column_name: str) -> bool: + """Return True if `table_name` has `column_name` (best-effort).""" if not _is_safe_identifier(table_name) or not _is_safe_identifier(column_name): logger.warning("Invalid identifier in table_has_column: %s.%s", table_name, column_name) return False @@ -233,6 +234,7 @@ def _ensure_column(db, table_name: str, column_name: str, definition: str) -> Re def ensure_columns_exist(db) -> Result[bool]: + """Ensure required columns exist in existing tables (best-effort).""" for table, columns in COLUMN_DEFINITIONS.items(): for column_name, definition in columns: result = _ensure_column(db, table, column_name, definition) @@ -243,6 +245,7 @@ def ensure_columns_exist(db) -> Result[bool]: def ensure_tables_exist(db) -> Result[bool]: + """Ensure base schema tables exist (idempotent).""" logger.info("Ensuring tables exist...") result = db.executescript(SCHEMA_V1) if not result.ok: @@ -251,6 +254,7 @@ def ensure_tables_exist(db) -> Result[bool]: def ensure_indexes_and_triggers(db) -> Result[bool]: + """Ensure indexes/triggers exist and repair FTS metadata (best-effort).""" logger.info("Ensuring indexes/triggers exist...") result = db.executescript(INDEXES_AND_TRIGGERS) if not result.ok: diff --git a/backend/adapters/db/sqlite.py b/backend/adapters/db/sqlite.py index 1d472db0..e5073c35 100644 --- a/backend/adapters/db/sqlite.py +++ b/backend/adapters/db/sqlite.py @@ -493,6 +493,7 @@ class Sqlite: raise def execute(self, query: str, params: Optional[tuple] = None, fetch: bool = False) -> Result[Any]: + """Execute SQL on the DB loop thread (sync).""" try: # Sync callers may be inside `transaction()` (token lives on the caller thread). # Pass it explicitly because DB work runs on the loop thread. @@ -502,14 +503,17 @@ class Sqlite: return Result.Err(ErrorCode.DB_ERROR, str(exc)) def query(self, sql: str, params: Optional[tuple] = None) -> Result[List[Dict[str, Any]]]: + """Execute a SELECT query and return rows (sync).""" return self.execute(sql, params, fetch=True) async def aexecute(self, query: str, params: Optional[tuple] = None, fetch: bool = False) -> Result[Any]: + """Execute SQL on the DB loop thread (async).""" token = _TX_TOKEN.get() fut = self._loop_thread.submit(self._execute_async(query, params, fetch, tx_token=token)) return await asyncio.wrap_future(fut) async def aquery(self, sql: str, params: Optional[tuple] = None) -> Result[List[Dict[str, Any]]]: + """Execute a SELECT query and return rows (async).""" return await self.aexecute(sql, params, fetch=True) def query_in( @@ -556,6 +560,7 @@ class Sqlite: values: List[Any], additional_params: Optional[tuple] = None, ) -> Result[List[Dict[str, Any]]]: + """Async variant of `query_in()` for IN-clause queries.""" if not values: return Result.Ok([]) if not isinstance(values, (list, tuple)): @@ -660,6 +665,7 @@ class Sqlite: raise def executemany(self, query: str, params_list: List[Tuple]) -> Result[int]: + """Execute a parameterized statement over multiple param tuples (sync).""" try: token = self._tx_token() return self._loop_thread.run(self._executemany_async(query, params_list, tx_token=token)) @@ -667,6 +673,7 @@ class Sqlite: return Result.Err(ErrorCode.DB_ERROR, str(exc)) async def aexecutemany(self, query: str, params_list: List[Tuple]) -> Result[int]: + """Execute a parameterized statement over multiple param tuples (async).""" token = _TX_TOKEN.get() fut = self._loop_thread.submit(self._executemany_async(query, params_list, tx_token=token)) return await asyncio.wrap_future(fut) @@ -713,6 +720,7 @@ class Sqlite: return Result.Err(ErrorCode.DB_ERROR, str(exc)) def executescript(self, script: str) -> Result[bool]: + """Execute a multi-statement SQL script (sync).""" try: token = self._tx_token() return self._loop_thread.run(self._executescript_async(script, tx_token=token)) @@ -720,6 +728,7 @@ class Sqlite: return Result.Err(ErrorCode.DB_ERROR, str(exc)) async def aexecutescript(self, script: str) -> Result[bool]: + """Execute a multi-statement SQL script (async).""" token = _TX_TOKEN.get() fut = self._loop_thread.submit(self._executescript_async(script, tx_token=token)) return await asyncio.wrap_future(fut) @@ -739,16 +748,19 @@ class Sqlite: await self._release_connection_async(conn) def vacuum(self) -> Result[bool]: + """Run SQLite VACUUM (sync).""" try: return self._loop_thread.run(self._vacuum_async()) except Exception as exc: return Result.Err(ErrorCode.DB_ERROR, str(exc)) async def avacuum(self) -> Result[bool]: + """Run SQLite VACUUM (async).""" fut = self._loop_thread.submit(self._vacuum_async()) return await asyncio.wrap_future(fut) def has_table(self, table_name: str) -> bool: + """Return True if `table_name` exists in sqlite_master.""" result = self.execute( "SELECT name FROM sqlite_master WHERE type='table' AND name = ?", (table_name,), @@ -757,6 +769,7 @@ class Sqlite: return bool(result.ok and result.data and len(result.data) > 0) def get_schema_version(self) -> int: + """Get the schema version from the `metadata` table (0 if missing).""" if not self.has_table("metadata"): return 0 @@ -773,6 +786,7 @@ class Sqlite: return 0 def set_schema_version(self, version: int) -> Result[bool]: + """Set the schema version in the `metadata` table.""" return self.execute( "INSERT OR REPLACE INTO metadata (key, value) VALUES ('schema_version', ?)", (str(version),), @@ -925,6 +939,7 @@ class Sqlite: pass def close(self): + """Close connections and stop the DB loop thread (sync).""" try: self._loop_thread.run(self._close_all_async()) except Exception as exc: @@ -933,6 +948,7 @@ class Sqlite: self._loop_thread.stop() async def aclose(self): + """Close connections and stop the DB loop thread (async).""" fut = self._loop_thread.submit(self._close_all_async()) try: await asyncio.wrap_future(fut) @@ -953,6 +969,7 @@ class Sqlite: @contextmanager def transaction(self, mode: str = "immediate"): + """Context manager for a DB transaction (sync).""" # Back-compat sync transaction: routes token via thread-local. begin_res = self._loop_thread.run(self._begin_tx_async(mode)) if not begin_res.ok or not begin_res.data: @@ -980,6 +997,7 @@ class Sqlite: @asynccontextmanager async def atransaction(self, mode: str = "immediate"): + """Async context manager for a DB transaction.""" begin_res = await self._begin_tx_for_async(mode) if not begin_res.ok or not begin_res.data: raise RuntimeError(str(begin_res.error or "Failed to begin transaction")) diff --git a/backend/config.py b/backend/config.py index ad840581..9ee563ba 100644 --- a/backend/config.py +++ b/backend/config.py @@ -51,6 +51,7 @@ TOOL_LOCATIONS = { } def get_tool_paths(): + """Return the resolved external tool executable paths.""" return TOOL_LOCATIONS.copy() # Media probe backend setting diff --git a/backend/custom_roots.py b/backend/custom_roots.py index 76a130af..e489cd07 100644 --- a/backend/custom_roots.py +++ b/backend/custom_roots.py @@ -102,6 +102,7 @@ def _write_store(data: dict) -> Result[bool]: def list_custom_roots() -> Result[List[Dict[str, Any]]]: + """List configured custom root directories (id/path/label/created_at).""" with _LOCK: store = _read_store() roots = store.get("roots") or [] @@ -125,6 +126,7 @@ def list_custom_roots() -> Result[List[Dict[str, Any]]]: def add_custom_root(path: str, label: Optional[str] = None) -> Result[Dict[str, Any]]: + """Add a custom root directory, or return the existing one if present.""" normalized = _normalize_dir_path(path) if not normalized: return Result.Err("INVALID_INPUT", "Invalid path") @@ -167,6 +169,7 @@ def add_custom_root(path: str, label: Optional[str] = None) -> Result[Dict[str, def remove_custom_root(root_id: str) -> Result[bool]: + """Remove a custom root by id.""" rid = str(root_id or "").strip() if not rid: return Result.Err("INVALID_INPUT", "Missing root_id") @@ -186,6 +189,7 @@ def remove_custom_root(root_id: str) -> Result[bool]: def resolve_custom_root(root_id: str) -> Result[Path]: + """Resolve a custom root id to a validated directory path.""" rid = str(root_id or "").strip() if not rid: return Result.Err("INVALID_INPUT", "Missing root_id") diff --git a/backend/features/collections/service.py b/backend/features/collections/service.py index 969d331d..de2950bb 100644 --- a/backend/features/collections/service.py +++ b/backend/features/collections/service.py @@ -96,6 +96,7 @@ def _collection_path(collection_id: str) -> Optional[Path]: @dataclass(frozen=True) class CollectionSummary: + """Lightweight summary row for collection listings.""" id: str name: str count: int @@ -103,6 +104,8 @@ class CollectionSummary: class CollectionsService: + """Filesystem-backed collections service (JSON files under `COLLECTIONS_DIR`).""" + def __init__(self, base_dir: Optional[str | Path] = None): self._base = Path(base_dir) if base_dir is not None else Path(COLLECTIONS_DIR_PATH) self._lock = threading.Lock() @@ -112,6 +115,7 @@ class CollectionsService: pass def list(self) -> Result[List[Dict[str, Any]]]: + """List collections with basic metadata and item counts.""" try: base = self._base.resolve(strict=False) except Exception as exc: @@ -139,6 +143,7 @@ class CollectionsService: return Result.Ok([i.__dict__ for i in items]) def create(self, name: str) -> Result[Dict[str, Any]]: + """Create a new empty collection.""" cname = _safe_name(name) if not cname: return Result.Err(ErrorCode.INVALID_INPUT, "Missing collection name") @@ -166,6 +171,7 @@ class CollectionsService: return Result.Ok({"id": cid, "name": cname}) def get(self, collection_id: str) -> Result[Dict[str, Any]]: + """Get a collection by id (including items).""" path = _collection_path(collection_id) if not path: return Result.Err(ErrorCode.INVALID_INPUT, "Invalid collection id") @@ -193,6 +199,7 @@ class CollectionsService: ) def delete(self, collection_id: str) -> Result[bool]: + """Delete a collection file by id.""" path = _collection_path(collection_id) if not path: return Result.Err(ErrorCode.INVALID_INPUT, "Invalid collection id") @@ -228,6 +235,7 @@ class CollectionsService: return path, data, None def add_assets(self, collection_id: str, assets: List[Dict[str, Any]]) -> Result[Dict[str, Any]]: + """Add assets (by filepath) to a collection (deduplicated, bounded).""" if not isinstance(assets, list) or not assets: return Result.Err(ErrorCode.INVALID_INPUT, "No assets provided") @@ -310,6 +318,7 @@ class CollectionsService: ) def remove_filepaths(self, collection_id: str, filepaths: List[str]) -> Result[Dict[str, Any]]: + """Remove items from a collection by filepath.""" if not isinstance(filepaths, list) or not filepaths: return Result.Err(ErrorCode.INVALID_INPUT, "No filepaths provided") targets = set(_normalize_fp(str(p)) for p in filepaths if str(p or "").strip()) diff --git a/backend/features/index/service.py b/backend/features/index/service.py index f1805197..17a2d851 100644 --- a/backend/features/index/service.py +++ b/backend/features/index/service.py @@ -197,6 +197,7 @@ class IndexService: return await self._searcher.date_histogram_scoped(roots, month_start, month_end, filters) async def get_asset(self, asset_id: int) -> Result[Optional[Dict[str, Any]]]: + """Fetch a single asset row by id.""" # Async path: return the DB row; deep "self-heal" is handled by scan/enrich flows. return await self._searcher.get_asset(asset_id) diff --git a/backend/features/index/watcher.py b/backend/features/index/watcher.py index e9f9518f..9d35ca70 100644 --- a/backend/features/index/watcher.py +++ b/backend/features/index/watcher.py @@ -35,6 +35,7 @@ class DirectoryWatcher: return threading.Thread(target=self._run, daemon=True) def start(self): + """Start the background watcher thread (idempotent).""" with self._lock: if self._thread is not None and self._thread.is_alive(): return @@ -47,6 +48,7 @@ class DirectoryWatcher: logger.info("File watcher started", extra={"directories": [str(d) for d in self.directories]}) def stop(self): + """Request the watcher thread to stop and join it (best-effort).""" self._stop_event.set() with self._lock: @@ -78,6 +80,7 @@ class DirectoryWatcher: logger.info("File watcher stopped successfully") def is_alive(self) -> bool: + """Return True if the watcher thread is currently running.""" with self._lock: return bool(self._thread and self._thread.is_alive()) diff --git a/backend/features/tags/sync.py b/backend/features/tags/sync.py index 314e4d0a..46c32e06 100644 --- a/backend/features/tags/sync.py +++ b/backend/features/tags/sync.py @@ -121,6 +121,7 @@ def write_exif_rating_tags(exiftool: ExifTool, file_path: str, rating: int, tags @dataclass(frozen=True) class RatingTagsSyncTask: + """Coalesced update request for a single file.""" file_path: str rating: int tags: List[str] @@ -277,6 +278,7 @@ class RatingTagsSyncWorker: self._thread.start() def stop(self, timeout: float = 2.0) -> None: + """Stop the background worker thread (best-effort).""" try: self._stop = True self._event.set() @@ -285,6 +287,7 @@ class RatingTagsSyncWorker: logger.debug("RatingTagsSyncWorker stop failed: %s", exc) def enqueue(self, file_path: str, rating: int, tags: List[str], mode: str) -> None: + """Queue a rating/tags update (coalesced per filepath).""" mode_norm = str(mode or "off").strip().lower() # Backward compatible: "sidecar"/"both" now map to "on" (no sidecar writes). if mode_norm in ("sidecar", "both", "true", "1", "yes", "enabled", "enable", "on"): diff --git a/backend/observability.py b/backend/observability.py index fea64eb7..1b880f96 100644 --- a/backend/observability.py +++ b/backend/observability.py @@ -156,6 +156,7 @@ def _should_log(request: web.Request, *, status: Optional[int], duration_ms: flo @web.middleware async def request_context_middleware(request: web.Request, handler): + """Add request-id correlation and lightweight request logging context.""" if _env_flag("MJR_OBS_DISABLE", default=False): return await handler(request) @@ -258,6 +259,7 @@ def ensure_observability(app: web.Application) -> None: def build_request_log_fields(request: web.Request, response_status: Optional[int] = None) -> Dict[str, Any]: + """Build a JSON-serializable dict of request/response fields for logs.""" stats = request.get("mjr_stats") return { "request_id": request.get("mjr_request_id"), diff --git a/backend/routes/core/paths.py b/backend/routes/core/paths.py index 0871b866..f98f4bb7 100644 --- a/backend/routes/core/paths.py +++ b/backend/routes/core/paths.py @@ -197,6 +197,39 @@ _ALLOWED_VIEW_EXTS = { def _guess_content_type_for_file(path: Path) -> str: + """ + Best-effort content-type for media serving. + + Note: On Windows, `mimetypes` may not include modern media types (webp/webm/avif) + depending on registry state. We keep a small explicit mapping for viewer-critical + formats to ensure browsers can decode them. + """ + try: + ext = str(path.suffix or "").lower() + if ext: + known = { + # Images + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".gif": "image/gif", + ".webp": "image/webp", + ".bmp": "image/bmp", + ".tif": "image/tiff", + ".tiff": "image/tiff", + ".avif": "image/avif", + # Videos + ".mp4": "video/mp4", + ".m4v": "video/x-m4v", + ".webm": "video/webm", + ".mov": "video/quicktime", + ".mkv": "video/x-matroska", + ".avi": "video/x-msvideo", + } + if ext in known: + return known[ext] + except Exception: + pass try: ct, _ = mimetypes.guess_type(str(path)) if ct: diff --git a/backend/routes/handlers/assets.py b/backend/routes/handlers/assets.py index dfdf3535..018e3eab 100644 --- a/backend/routes/handlers/assets.py +++ b/backend/routes/handlers/assets.py @@ -89,6 +89,7 @@ def _delete_file_best_effort(path: Path) -> Result[bool]: def register_asset_routes(routes: web.RouteTableDef) -> None: + """Register asset CRUD routes (get, delete, rename).""" async def _resolve_or_create_asset_id( *, services: dict, diff --git a/backend/routes/handlers/batch_zip.py b/backend/routes/handlers/batch_zip.py index f0b3f04c..9c033765 100644 --- a/backend/routes/handlers/batch_zip.py +++ b/backend/routes/handlers/batch_zip.py @@ -159,6 +159,7 @@ def _resolve_item_path(item: Dict[str, Any]) -> Optional[Path]: def register_batch_zip_routes(routes: web.RouteTableDef) -> None: + """Register batch-zip creation and download routes.""" @routes.post("/mjr/am/batch-zip") async def create_batch_zip(request: web.Request) -> web.Response: csrf = _csrf_error(request) diff --git a/backend/routes/handlers/calendar.py b/backend/routes/handlers/calendar.py index ad4da197..b41072b1 100644 --- a/backend/routes/handlers/calendar.py +++ b/backend/routes/handlers/calendar.py @@ -40,6 +40,7 @@ def _month_bounds(month_value: str) -> Result[tuple[int, int]]: def register_calendar_routes(routes: web.RouteTableDef) -> None: + """Register calendar/date-histogram routes.""" @routes.get("/mjr/am/date-histogram") async def date_histogram(request: web.Request): """ diff --git a/backend/routes/handlers/collections.py b/backend/routes/handlers/collections.py index 74b61eff..201ef92b 100644 --- a/backend/routes/handlers/collections.py +++ b/backend/routes/handlers/collections.py @@ -77,6 +77,7 @@ def _minimal_asset_from_item(item: Dict[str, Any]) -> Dict[str, Any]: def register_collections_routes(routes: web.RouteTableDef) -> None: + """Register collection management routes.""" @routes.get("/mjr/am/collections") async def list_collections(request): try: diff --git a/backend/routes/handlers/custom_roots.py b/backend/routes/handlers/custom_roots.py index 82a31117..8e6e5069 100644 --- a/backend/routes/handlers/custom_roots.py +++ b/backend/routes/handlers/custom_roots.py @@ -18,6 +18,7 @@ logger = get_logger(__name__) def register_custom_roots_routes(routes: web.RouteTableDef) -> None: + """Register custom root directory routes.""" @routes.get("/mjr/am/custom-roots") async def get_custom_roots(request): result = list_custom_roots() diff --git a/backend/routes/handlers/db_maintenance.py b/backend/routes/handlers/db_maintenance.py index bbdb2f55..c755c519 100644 --- a/backend/routes/handlers/db_maintenance.py +++ b/backend/routes/handlers/db_maintenance.py @@ -13,6 +13,7 @@ logger = get_logger(__name__) def register_db_maintenance_routes(routes: web.RouteTableDef) -> None: + """Register database maintenance routes.""" @routes.post("/mjr/am/db/optimize") async def db_optimize(request: web.Request): """ @@ -49,4 +50,3 @@ def register_db_maintenance_routes(routes: web.RouteTableDef) -> None: return _json_response(Result.Err("DB_ERROR", safe_error_message(exc, "Database optimize failed"))) return _json_response(Result.Ok({"ran": steps})) - diff --git a/backend/routes/handlers/filesystem.py b/backend/routes/handlers/filesystem.py index cee9e7eb..b4b4c968 100644 --- a/backend/routes/handlers/filesystem.py +++ b/backend/routes/handlers/filesystem.py @@ -1,12 +1,15 @@ """ Filesystem operations: background scanning and file listing. """ + +from __future__ import annotations + import datetime import time import threading from collections import OrderedDict from pathlib import Path -from typing import Any, Dict, Optional +from typing import Any, Mapping, Optional from backend.shared import Result, classify_file, get_logger from backend.adapters.fs.list_cache_watcher import ensure_fs_list_cache_watching, get_fs_list_cache_token @@ -16,17 +19,17 @@ from ..core import _safe_rel_path, _is_within_root logger = get_logger(__name__) _BACKGROUND_SCAN_LOCK = threading.Lock() -_BACKGROUND_SCAN_LAST: Dict[str, float] = {} -_BACKGROUND_SCAN_FAILURES: list[dict] = [] +_BACKGROUND_SCAN_LAST: dict[str, float] = {} +_BACKGROUND_SCAN_FAILURES: list[dict[str, Any]] = [] _MAX_FAILURE_HISTORY = int(BG_SCAN_FAILURE_HISTORY_MAX) _SCAN_PENDING_LOCK = threading.Lock() -_SCAN_PENDING: "OrderedDict[str, Dict[str, Any]]" = OrderedDict() +_SCAN_PENDING: OrderedDict[str, dict[str, Any]] = OrderedDict() _SCAN_WORKER: Optional[threading.Thread] = None _SCAN_PENDING_MAX = int(SCAN_PENDING_MAX) _FS_LIST_CACHE_LOCK = threading.Lock() -_FS_LIST_CACHE: "OrderedDict[str, Dict[str, Any]]" = OrderedDict() +_FS_LIST_CACHE: OrderedDict[str, dict[str, Any]] = OrderedDict() def _kickoff_background_scan( @@ -48,11 +51,6 @@ def _kickoff_background_scan( try: key = f"{source}|{str(root_id or '')}|{str(directory)}" now = time.time() - with _BACKGROUND_SCAN_LOCK: - last = _BACKGROUND_SCAN_LAST.get(key, 0.0) - if now - last < float(min_interval_seconds): - return - _BACKGROUND_SCAN_LAST[key] = now except Exception: return @@ -66,12 +64,13 @@ def _kickoff_background_scan( "background_metadata": bool(background_metadata), } - def _ensure_worker(): + def _ensure_worker() -> None: global _SCAN_WORKER - if _SCAN_WORKER and _SCAN_WORKER.is_alive(): - return + with _SCAN_PENDING_LOCK: + if _SCAN_WORKER and _SCAN_WORKER.is_alive(): + return - def _worker_loop(): + def _worker_loop() -> None: while True: task = None with _SCAN_PENDING_LOCK: @@ -88,7 +87,7 @@ def _kickoff_background_scan( try: from ..core import _require_services - svc, error_result = _require_services() + svc, error_result = _require_services() # type: ignore[no-untyped-call] if error_result: _record_scan_failure(str(task.get("directory")), str(task.get("source")), "SERVICE_UNAVAILABLE", str(error_result.error or "")) continue @@ -122,18 +121,36 @@ def _kickoff_background_scan( pass try: - _SCAN_WORKER = threading.Thread(target=_worker_loop, name="mjr-bg-scan-worker", daemon=True) - _SCAN_WORKER.start() + worker = threading.Thread(target=_worker_loop, name="mjr-bg-scan-worker", daemon=True) except Exception: - _SCAN_WORKER = None + with _SCAN_PENDING_LOCK: + _SCAN_WORKER = None + return + + with _SCAN_PENDING_LOCK: + _SCAN_WORKER = worker + try: + worker.start() + except Exception: + with _SCAN_PENDING_LOCK: + if _SCAN_WORKER is worker: + _SCAN_WORKER = None try: _ensure_worker() - with _SCAN_PENDING_LOCK: - _SCAN_PENDING[key] = job - _SCAN_PENDING.move_to_end(key) - while len(_SCAN_PENDING) > _SCAN_PENDING_MAX: - _SCAN_PENDING.popitem(last=False) + with _BACKGROUND_SCAN_LOCK: + last = _BACKGROUND_SCAN_LAST.get(key, 0.0) + if now - last < float(min_interval_seconds): + return + + with _SCAN_PENDING_LOCK: + _SCAN_PENDING[key] = job + _SCAN_PENDING.move_to_end(key) + while len(_SCAN_PENDING) > _SCAN_PENDING_MAX: + _SCAN_PENDING.popitem(last=False) + + # Race fix: only update the throttle marker once the job is enqueued. + _BACKGROUND_SCAN_LAST[key] = now except Exception: return @@ -160,7 +177,7 @@ def _fs_cache_get_or_build( target_dir_resolved: Path, asset_type: str, root_id: Optional[str], -) -> Result[Dict[str, Any]]: +) -> Result[dict[str, Any]]: try: if FS_LIST_CACHE_WATCHDOG: ensure_fs_list_cache_watching(str(base)) @@ -198,7 +215,7 @@ def _fs_cache_get_or_build( # If TTL expired, rebuild even if directory mtime did not change. Some filesystems # do not bump dir mtime when file contents change, which would otherwise cause stale listings. - entries = [] + entries: list[dict[str, Any]] = [] try: for entry in target_dir_resolved.iterdir(): if not entry.is_file(): @@ -262,9 +279,9 @@ async def _list_filesystem_assets( offset: int, asset_type: str, root_id: Optional[str] = None, - filters: Optional[dict] = None, + filters: Optional[Mapping[str, Any]] = None, index_service: Optional[Any] = None, -) -> Result[dict]: +) -> Result[dict[str, Any]]: try: base = root_dir.resolve() except OSError as exc: @@ -287,15 +304,14 @@ async def _list_filesystem_assets( q_lower = q.lower() browse_all = q == "*" or q == "" - filter_kind = str(filters.get("kind") or "").strip().lower() if isinstance(filters, dict) else "" - filter_min_rating = int(filters.get("min_rating") or 0) if isinstance(filters, dict) else 0 - filter_workflow_only = bool(filters.get("has_workflow")) if isinstance(filters, dict) else False - filter_mtime_start = ( - int(filters.get("mtime_start")) if isinstance(filters, dict) and filters.get("mtime_start") is not None else None - ) - filter_mtime_end = ( - int(filters.get("mtime_end")) if isinstance(filters, dict) and filters.get("mtime_end") is not None else None - ) + filter_kind = str(filters.get("kind") or "").strip().lower() if filters is not None else "" + filter_min_rating = int(filters.get("min_rating") or 0) if filters is not None else 0 + filter_workflow_only = bool(filters.get("has_workflow")) if filters is not None else False + + mtime_start_raw = filters.get("mtime_start") if filters is not None else None + mtime_end_raw = filters.get("mtime_end") if filters is not None else None + filter_mtime_start = int(mtime_start_raw) if mtime_start_raw is not None else None + filter_mtime_end = int(mtime_end_raw) if mtime_end_raw is not None else None # Removed the early return that was causing filters to return empty results # The filtering will now be handled properly below after metadata enrichment @@ -359,7 +375,7 @@ async def _list_filesystem_assets( # Apply filters after enrichment. # Keep memory predictable by building only the requested page while still computing `total`. total = 0 - paged = [] + paged: list[dict[str, Any]] = [] start = max(0, int(offset or 0)) end = start + int(limit or 0) if int(limit or 0) > 0 else None diff --git a/backend/routes/handlers/health.py b/backend/routes/handlers/health.py index 85803ffe..e21de3b7 100644 --- a/backend/routes/handlers/health.py +++ b/backend/routes/handlers/health.py @@ -24,6 +24,7 @@ from ..core import _json_response, _require_services, _csrf_error, _read_json def register_health_routes(routes: web.RouteTableDef) -> None: + """Register health and diagnostics routes.""" @routes.get("/mjr/am/health") async def health(request): """Get health status.""" diff --git a/backend/routes/handlers/metadata.py b/backend/routes/handlers/metadata.py index 8d83ce7c..85c23ab2 100644 --- a/backend/routes/handlers/metadata.py +++ b/backend/routes/handlers/metadata.py @@ -31,6 +31,7 @@ except Exception: def register_metadata_routes(routes: web.RouteTableDef) -> None: + """Register metadata extraction routes.""" @routes.get("/mjr/am/metadata") async def get_metadata(request): """ diff --git a/backend/routes/handlers/scan.py b/backend/routes/handlers/scan.py index e89f754c..e22aa25c 100644 --- a/backend/routes/handlers/scan.py +++ b/backend/routes/handlers/scan.py @@ -214,6 +214,7 @@ def _files_equal_content(src: Path, dst: Path, *, chunk_size: int = _FILE_COMPAR @dataclass(frozen=True) class StageDestination: + """Resolved destination for staging (path + whether it was reused).""" path: Path reused_existing: bool @@ -258,6 +259,7 @@ def _resolve_scan_root(directory: str) -> Result[Path]: def register_scan_routes(routes: web.RouteTableDef) -> None: + """Register scan/index/stage/open-in-folder routes.""" @routes.post("/mjr/am/scan") async def scan_directory(request): """ diff --git a/backend/routes/handlers/search.py b/backend/routes/handlers/search.py index 13658012..dddfc0da 100644 --- a/backend/routes/handlers/search.py +++ b/backend/routes/handlers/search.py @@ -71,6 +71,7 @@ def _date_bounds_for_exact(value): def register_search_routes(routes: web.RouteTableDef) -> None: + """Register listing/search routes.""" @routes.get("/mjr/am/list") async def list_assets(request): """ diff --git a/backend/routes/handlers/viewer.py b/backend/routes/handlers/viewer.py index 5f9fb997..dd06e843 100644 --- a/backend/routes/handlers/viewer.py +++ b/backend/routes/handlers/viewer.py @@ -25,6 +25,7 @@ logger = get_logger(__name__) def register_viewer_routes(routes: web.RouteTableDef) -> None: + """Register viewer info and file-serving routes.""" @routes.get("/mjr/am/viewer/info") async def viewer_info(request: web.Request): """ @@ -88,4 +89,3 @@ def register_viewer_routes(routes: web.RouteTableDef) -> None: info["mime"] = None return _json_response(Result.Ok(info)) - diff --git a/backend/routes/registry.py b/backend/routes/registry.py index 0057d721..a3682d7e 100644 --- a/backend/routes/registry.py +++ b/backend/routes/registry.py @@ -2,17 +2,47 @@ Route registration system. Coordinates all route handlers and registers them with PromptServer or aiohttp app. """ + +from __future__ import annotations + from aiohttp import web from backend.shared import get_logger from backend.observability import ensure_observability +from typing import Awaitable, Callable, ClassVar, Protocol, cast + + +class _PromptServerInstance(Protocol): + routes: web.RouteTableDef + app: web.Application | None + + +class _PromptServer(Protocol): + instance: ClassVar[_PromptServerInstance] try: - from server import PromptServer # type: ignore -except Exception: - class _PromptServerStub: - instance = type("PromptServerInstance", (), {"routes": web.RouteTableDef()})() + # IMPORTANT (ComfyUI compatibility): + # Never `import server` here. Importing ComfyUI's `server.py` triggers a cascade + # (nodes -> torch -> cuda init) which can hard-crash the interpreter in non-ComfyUI + # contexts (unit tests, docs scripts, standalone runs) or on misconfigured CUDA. + # + # In the real ComfyUI runtime, `server` is already imported and lives in + # `sys.modules`, so we can safely reuse it without re-importing. + import sys - PromptServer = _PromptServerStub # type: ignore + _server_mod = sys.modules.get("server") + if _server_mod is None or not hasattr(_server_mod, "PromptServer"): + raise ImportError("ComfyUI server not loaded") + + PromptServer = cast(type[_PromptServer], getattr(_server_mod, "PromptServer")) +except Exception: + class _PromptServerInstanceStub: + routes: web.RouteTableDef = web.RouteTableDef() + app: web.Application | None = None + + class _PromptServerStub: + instance: ClassVar[_PromptServerInstanceStub] = _PromptServerInstanceStub() + + PromptServer = cast(type[_PromptServer], _PromptServerStub) from .handlers import ( register_health_routes, @@ -32,7 +62,11 @@ logger = get_logger(__name__) @web.middleware -async def security_headers_middleware(request, handler): +async def security_headers_middleware( + request: web.Request, + handler: Callable[[web.Request], Awaitable[web.StreamResponse]], +) -> web.StreamResponse: + """Apply strict security headers to Majoor API responses only.""" response = await handler(request) # Only apply to Majoor API endpoints. Applying `nosniff` / CSP globally can break @@ -60,7 +94,10 @@ async def security_headers_middleware(request, handler): @web.middleware -async def api_versioning_middleware(request, handler): +async def api_versioning_middleware( + request: web.Request, + handler: Callable[[web.Request], Awaitable[web.StreamResponse]], +) -> web.StreamResponse: """ Support versioned routes without breaking existing clients. diff --git a/backend/routes_compat.py b/backend/routes_compat.py index fc50af71..100fa2f6 100644 --- a/backend/routes_compat.py +++ b/backend/routes_compat.py @@ -13,6 +13,10 @@ same name. This shim is deliberately named `backend.routes_compat` so the canonical entry-point remains the `backend.routes` *package*. """ +from __future__ import annotations + +from typing import Protocol, cast + # Import the modular routes system which auto-registers everything. from .routes import ( # noqa: F401 register_routes, @@ -48,8 +52,17 @@ from .shared import Result, get_logger # noqa: F401 logger = get_logger(__name__) # Expose folder_paths stub for compatibility. +class _FolderPaths(Protocol): + def get_input_directory(self) -> str: ... + + def get_output_directory(self) -> str: ... + + try: - import folder_paths # type: ignore + import folder_paths as _folder_paths + + folder_paths: _FolderPaths + folder_paths = cast(_FolderPaths, _folder_paths) except Exception: # pragma: no cover from pathlib import Path @@ -62,18 +75,35 @@ except Exception: # pragma: no cover def get_output_directory() -> str: return str((Path(__file__).resolve().parents[2] / "output").resolve()) - folder_paths = _FolderPathsStub() # type: ignore + folder_paths = cast(_FolderPaths, _FolderPathsStub()) # Expose PromptServer stub for compatibility. try: - from server import PromptServer # type: ignore + # Do not import ComfyUI's `server.py` here: importing it can trigger heavy + # initialization (nodes/torch/cuda) and may hard-crash outside the real + # ComfyUI runtime. Prefer reusing the already-loaded module when present. + import sys + + _server_mod = sys.modules.get("server") + if _server_mod is None or not hasattr(_server_mod, "PromptServer"): + raise ImportError("ComfyUI server not loaded") + + from aiohttp import web + + class _PromptServerInstance(Protocol): + routes: web.RouteTableDef + + class _PromptServer(Protocol): + instance: _PromptServerInstance + + PromptServer = cast(_PromptServer, getattr(_server_mod, "PromptServer")) except Exception: # pragma: no cover from aiohttp import web class _PromptServerStub: instance = type("PromptServerInstance", (), {"routes": web.RouteTableDef()})() - PromptServer = _PromptServerStub # type: ignore + PromptServer = _PromptServerStub __all__ = [ # Main functions diff --git a/backend/settings.py b/backend/settings.py index 199feeee..b3825a7d 100644 --- a/backend/settings.py +++ b/backend/settings.py @@ -93,6 +93,7 @@ class AppSettings: return Result.Ok(v) def get_probe_backend(self) -> str: + """Return the configured media probe backend mode.""" with self._lock: current_version = self._get_settings_version() cached = self._cache.get(_PROBE_BACKEND_KEY) @@ -113,6 +114,7 @@ class AppSettings: return mode def set_probe_backend(self, mode: str) -> Result[str]: + """Persist the media probe backend mode and bump the settings version.""" normalized = (mode or "").strip().lower() if not normalized: normalized = self._default_probe_mode diff --git a/backend/shared.py b/backend/shared.py index 7b2b8227..76e85ff5 100644 --- a/backend/shared.py +++ b/backend/shared.py @@ -1,11 +1,25 @@ -"""Backend-facing alias for shared utilities.""" +"""Backend-facing alias for shared utilities. -try: - # When loaded by ComfyUI as a package (e.g. .backend.*) - from .. import shared as _root_shared # type: ignore -except Exception: - # When running tests or scripts that import `backend.*` as a top-level module - import shared as _root_shared # type: ignore +This file exists because ComfyUI can load custom nodes either: +- as a pseudo-package via file paths under `custom_nodes/`, or +- as top-level modules when running tests/scripts. + +All imports here are therefore best-effort and must not crash the UI. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + import shared as _root_shared +else: + try: + # When loaded by ComfyUI as a package (e.g. .backend.*) + from .. import shared as _root_shared # type: ignore + except Exception: + # When running tests or scripts that import `backend.*` as a top-level module + import shared as _root_shared # type: ignore Result = _root_shared.Result ErrorCode = _root_shared.ErrorCode @@ -18,13 +32,17 @@ FileKind = _root_shared.FileKind MetadataQuality = _root_shared.MetadataQuality IndexMode = _root_shared.IndexMode MetadataMode = _root_shared.MetadataMode -try: - from ..shared.types import EXTENSIONS as EXTENSIONS # type: ignore -except Exception: + +if TYPE_CHECKING: + from shared.types import EXTENSIONS as EXTENSIONS +else: try: - from shared.types import EXTENSIONS as EXTENSIONS # type: ignore + from ..shared.types import EXTENSIONS as EXTENSIONS # type: ignore except Exception: - EXTENSIONS = {} # type: ignore + try: + from shared.types import EXTENSIONS as EXTENSIONS # type: ignore + except Exception: + EXTENSIONS: dict[str, Any] = {} __all__ = _root_shared.__all__ + [ "Result", diff --git a/docs/THREAT_MODEL.md b/docs/THREAT_MODEL.md new file mode 100644 index 00000000..a06341b2 --- /dev/null +++ b/docs/THREAT_MODEL.md @@ -0,0 +1,75 @@ +# Threat Model — Majoor Assets Manager + +Date: 2026-01-24 + +## Scope +This model covers the Majoor Assets Manager backend routes under `/mjr/am/*` and the associated local filesystem/database operations. + +## Assets to Protect +- User files on disk (input/output/custom roots). +- SQLite index database and collections JSON files. +- ComfyUI server availability (avoid crashes / runaway CPU / locks). +- Privacy of metadata (prompts/workflows embedded in images/videos). + +## Trust Boundaries +- **Browser/UI** → **ComfyUI HTTP server**: untrusted inputs via query params and JSON bodies. +- **Extension backend** → **Filesystem**: path traversal and symlink/junction concerns. +- **Extension backend** → **External tools** (`exiftool`, `ffprobe`): command execution and parsing. +- **Extension backend** → **SQLite**: injection/locking/corruption risks. + +## Threats & Mitigations + +### 1) Path Traversal / Unauthorized File Access +**Threat:** An attacker (or malicious workflow) calls `/mjr/am/*` with paths pointing outside allowed roots. + +**Mitigations:** +- Normalize paths and reject invalid/unsafe paths (`_safe_rel_path`, `_is_within_root`, allow-lists). +- Custom roots are resolved and validated (root-id → canonical directory). +- Avoid exposing arbitrary “open file” primitives; use explicit allow-listed actions. + +### 2) Symlink/Junction Escape (TOCTOU) +**Threat:** A path initially validated as within root later resolves to a different location (symlink swap). + +**Mitigations:** +- Resolve roots/targets with `resolve(strict=True)` where possible. +- Validate after resolution and before use. +- Treat mid-scan disappearance as a benign failure and continue (best-effort). + +### 3) CSRF / Cross-Site Requests +**Threat:** A local web page triggers state-changing endpoints (delete/rename/stage) against `localhost`. + +**Mitigations:** +- CSRF checks on state-changing routes. +- Prefer returning `Result.Err("CSRF", ...)` instead of throwing. + +### 4) Denial of Service (Large Listings / Heavy Scans) +**Threat:** Repeated list/index requests cause CPU/disk thrash. + +**Mitigations:** +- Bounded queues for background work (scan pending max). +- Rate limiting for expensive endpoints. +- TTL-based filesystem listing cache with bounded size. + +### 5) External Tool Execution Risks +**Threat:** Tool invocation on untrusted media can be slow or crash; tool may be missing. + +**Mitigations:** +- Timeouts and best-effort fallbacks (`code="DEGRADED"`). +- Never crash UI: return `Result.Err/Ok` with degraded metadata. +- Avoid shell string interpolation; prefer argv lists. + +### 6) Data Corruption / Concurrent Writes +**Threat:** Concurrent updates corrupt JSON stores or SQLite schema state. + +**Mitigations:** +- Atomic writes for JSON stores (write tmp + replace). +- SQLite locking strategy and bounded transactions. + +## Residual Risk +- Localhost endpoints are inherently accessible to local malware. +- Media parsing remains a complex attack surface (mitigated by timeouts and least-privilege execution). + +## Non-Goals +- Remote multi-user authentication/authorization (ComfyUI is typically local-only). +- Protecting against a fully compromised host OS. + diff --git a/docs/adr/0001-error-handling-result.md b/docs/adr/0001-error-handling-result.md new file mode 100644 index 00000000..ab076e02 --- /dev/null +++ b/docs/adr/0001-error-handling-result.md @@ -0,0 +1,34 @@ +# ADR 0001: Error Handling via `Result` (No UI Exceptions) + +Date: 2026-01-24 + +## Status +Accepted + +## Context +ComfyUI loads custom nodes into a long-running server process. An uncaught exception inside an extension can: +- break the UI (failed endpoints / broken sidebar), +- force restarts, and/or +- hide partial but useful results (e.g. missing `exiftool`, partial metadata). + +We need a consistent way to report *expected* failures (tool missing, unsupported format, partial metadata) without crashing the UI or relying on HTTP status codes. + +## Decision +- Backend services and adapters return `Result[T]` instead of raising for expected errors. +- Route handlers **must not** let exceptions bubble to ComfyUI’s request handling. Any unexpected exception is caught and converted into `Result.Err(...)`. +- For business/expected errors, HTTP routes return **200** with `{ ok: false, code, error, meta }`. +- Non-200 status codes are reserved for true server bugs (uncaught exceptions), and we aim for “never” by design. + +## Consequences +### Positive +- More resilient UI: missing tools or partial metadata does not crash the extension. +- Clear client behavior: the JS client can treat every response as a `Result` payload. +- Easier observability: errors are structured by `code`. + +### Negative +- Requires discipline: any new code path must follow the `Result` pattern. +- Some integration tests must check `Result.ok` rather than HTTP status. + +## Notes +This ADR pairs with a frontend convention: the JS API client must not throw and should return `{ ok, data, error, code, meta }`. + diff --git a/docs/adr/0002-comfyui-server-import-safety.md b/docs/adr/0002-comfyui-server-import-safety.md new file mode 100644 index 00000000..5f743eac --- /dev/null +++ b/docs/adr/0002-comfyui-server-import-safety.md @@ -0,0 +1,26 @@ +# ADR 0002: Avoid Importing `ComfyUI/server.py` from Extension Code + +Date: 2026-01-24 + +## Status +Accepted + +## Context +Importing ComfyUI’s `server.py` can trigger heavy initialization (e.g. `nodes -> torch -> cuda init`). +In non-ComfyUI contexts (unit tests, documentation scripts, or misconfigured CUDA setups), importing `server.py` can cause hard crashes (native access violations) rather than a Python exception. + +The extension needs to register routes in ComfyUI, but also remain importable in test/script contexts. + +## Decision +- Do **not** `import server` (or `from server import PromptServer`) inside extension modules. +- When running inside ComfyUI, `server` is already imported; reuse it via `sys.modules["server"]`. +- When not running inside ComfyUI, fall back to a safe stub that provides a `routes` table for tests. + +## Consequences +### Positive +- Prevents hard crashes during import outside ComfyUI. +- Keeps unit tests and scripts safe and fast. + +### Negative +- Requires small amount of compatibility/protocol typing code (stubs/protocols). + diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 00000000..e5ac5853 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,8 @@ +# Architecture Decision Records (ADRs) + +This folder tracks non-trivial architectural decisions for Majoor Assets Manager. + +## Index +- `0001-error-handling-result.md` — Use `Result[T]` everywhere, no UI exceptions. +- `0002-comfyui-server-import-safety.md` — Never import `ComfyUI/server.py` from extension code. + diff --git a/js/features/viewer/mediaFactory.js b/js/features/viewer/mediaFactory.js index 25f08ba0..625716fe 100644 --- a/js/features/viewer/mediaFactory.js +++ b/js/features/viewer/mediaFactory.js @@ -22,6 +22,98 @@ export function createViewerMediaFactory({ const _safeCall = safeCall || defaultSafeCall; const _safeAddListener = safeAddListener || defaultSafeAddListener; + const _extOf = (asset) => { + try { + const ext = String(asset?.ext || "").trim().toLowerCase(); + if (ext) return ext.startsWith(".") ? ext : `.${ext}`; + } catch {} + try { + const name = String(asset?.filename || asset?.filepath || "").trim(); + const idx = name.lastIndexOf("."); + if (idx >= 0) return name.slice(idx).toLowerCase(); + } catch {} + return ""; + }; + + // Formats that may be animated (GIF, animated WebP). Rendering through the + // image processor (canvas) would only show a single frame. + const _shouldUseNativeImageEl = (asset) => { + const ext = _extOf(asset); + return ext === ".gif" || ext === ".webp"; + }; + + const _createNativeImageEl = (asset, url) => { + const img = document.createElement("img"); + img.className = "mjr-viewer-media"; + try { + if (asset?.id != null && img?.dataset) img.dataset.mjrAssetId = String(asset.id); + } catch {} + img.alt = String(asset?.filename || "") || "image"; + try { + img.decoding = "async"; + } catch {} + try { + img.loading = "eager"; + } catch {} + try { + img.draggable = false; + } catch {} + img.src = url; + img.style.cssText = ` + max-width: 100%; + max-height: 100%; + display: block; + transform: ${mediaTransform?.() || ""}; + transform-origin: center center; + `; + + const onLoad = () => { + try { + requestAnimationFrame(() => { + try { + if (!state?._userInteracted) { + updateMediaNaturalSize?.(); + clampPanToBounds?.(); + applyTransform?.(); + } + } catch {} + }); + } catch {} + try { + scheduleOverlayRedraw?.(); + } catch {} + }; + + const onError = () => { + try { + const canvas = document.createElement("canvas"); + canvas.className = "mjr-viewer-media"; + try { + if (asset?.id != null && canvas?.dataset) canvas.dataset.mjrAssetId = String(asset.id); + } catch {} + _seedNaturalSizeFromAsset(canvas, asset); + canvas.style.cssText = ` + max-width: 100%; + max-height: 100%; + display: block; + transform: ${mediaTransform?.() || ""}; + transform-origin: center center; + `; + drawMediaError(canvas, "Failed to load image"); + img.replaceWith(canvas); + } catch {} + }; + + try { + img.addEventListener("load", onLoad, { once: true }); + } catch {} + try { + img.addEventListener("error", onError, { once: true }); + } catch {} + + return img; + }; + const _seedNaturalSizeFromAsset = (canvas, asset) => { try { if (!canvas || !(canvas instanceof HTMLCanvasElement)) return; @@ -157,6 +249,10 @@ export function createViewerMediaFactory({ return container; } + if (_shouldUseNativeImageEl(asset)) { + return _createNativeImageEl(asset, url); + } + const canvas = document.createElement("canvas"); canvas.className = "mjr-viewer-media"; try { @@ -291,6 +387,10 @@ export function createViewerMediaFactory({ return wrap; } + if (_shouldUseNativeImageEl(asset)) { + return _createNativeImageEl(asset, url); + } + const canvas = document.createElement("canvas"); canvas.className = "mjr-viewer-media"; try { diff --git a/js/features/viewer/videoProcessor.js b/js/features/viewer/videoProcessor.js index d3b30422..37bf6ac1 100644 --- a/js/features/viewer/videoProcessor.js +++ b/js/features/viewer/videoProcessor.js @@ -385,7 +385,7 @@ export function createVideoProcessor({ const onError = () => { proc.ready = false; try { - drawMediaError(canvas, "Failed to load video"); + drawMediaError(canvas, "Failed to load video (unsupported codec/format?)"); } catch {} }; diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 00000000..657efacf --- /dev/null +++ b/mypy.ini @@ -0,0 +1,35 @@ +[mypy] +python_version = 3.10 +strict = True +warn_unused_configs = True +explicit_package_bases = True +namespace_packages = True + +# Type-check the core modules first (expand over time). +files = + shared, + backend/shared.py, + backend/routes/registry.py, + backend/routes_compat.py, + backend/routes/handlers/filesystem.py + +# Third-party libs in the ComfyUI embedded environment can be present/absent +# depending on installation mode. Keep mypy focused on our code. +ignore_missing_imports = True + +# Keep the rollout manageable: start strict on a small set of modules while the +# rest of the codebase is being annotated. +[mypy-backend.*] +ignore_errors = True + +[mypy-backend.shared] +ignore_errors = False + +[mypy-backend.routes.registry] +ignore_errors = False + +[mypy-backend.routes_compat] +ignore_errors = False + +[mypy-backend.routes.handlers.filesystem] +ignore_errors = False diff --git a/pyproject.toml b/pyproject.toml index d745fd4c..b5eaed15 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,8 +1,11 @@ [project] -name = "majoor-assetsmanager" +name = "ComfyUI-Majoor-AssetsManager" description = "Majoor Assets Manager is an advanced asset browser for ComfyUI that provides a comprehensive solution for managing, organizing, and viewing your generated assets. It integrates directly into ComfyUI, offering features like full-text search, metadata extraction, rating and tagging systems, and advanced viewing capabilities." version = "2.1.1" license = {file = "LICENSE"} +authors = [ + { name = "MajoorWaldi" }, +] # classifiers = [ # # For OS-independent nodes (works on all operating systems) # "Operating System :: OS Independent", @@ -20,9 +23,17 @@ license = {file = "LICENSE"} # "Environment :: GPU :: Apple Metal", # Apple Metal support # ] -dependencies = ["# ComfyUI-Majoor-AssetsManager Python Dependencies", "# Core web framework", "aiohttp>=3.8.0", "# Async SQLite (non-blocking DB adapter)", "aiosqlite>=0.20.0", "# Image processing and thumbnails", "# Updated for security patches (CVE-2023-50447, CVE-2024-28219)", "pillow>=11.0.0", "# Safe file deletion (recycle bin support)", "send2trash>=1.8.0", "# File system monitoring for auto-reindexing", "watchdog>=2.1.0", "# Windows-specific: Windows Property System metadata support", "# Only required on Windows; gracefully skipped on other platforms", "pywin32>=300; sys_platform == 'win32'"] +dependencies = [ + "aiohttp>=3.8.0", + "aiosqlite>=0.20.0", + "pillow>=11.0.0", + "send2trash>=1.8.0", + "watchdog>=2.1.0", + "pywin32>=300; sys_platform == 'win32'", +] [project.urls] +Homepage = "https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager" Repository = "https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager" # Used by Comfy Registry https://registry.comfy.org Documentation = "https://github.com/MajoorWaldi/ComfyUI-Majoor-AssetsManager/blob/main/user_guide.html" diff --git a/shared/log.py b/shared/log.py index 8e230120..a3552167 100644 --- a/shared/log.py +++ b/shared/log.py @@ -5,10 +5,10 @@ import json import logging from contextvars import ContextVar from datetime import datetime, timezone -from typing import Optional +from typing import Any, Final, Optional # Emoji indicators for log levels -EMOJI_MAP = { +EMOJI_MAP: Final[dict[str, str]] = { "DEBUG": "🔍", # Magnifying glass for debug "INFO": "ℹ️", # Info symbol "WARNING": "⚠️", # Warning sign @@ -18,24 +18,28 @@ EMOJI_MAP = { } # Global logger prefix -PREFIX = "📂 Majoor" +PREFIX: Final[str] = "📂 Majoor" request_id_var: ContextVar[str] = ContextVar("request_id", default="") class CorrelationFilter(logging.Filter): + """Inject `request_id` from `request_id_var` into each log record.""" + def filter(self, record: logging.LogRecord) -> bool: + """Attach `record.request_id` for correlation; always returns True.""" try: - record.request_id = request_id_var.get("") # type: ignore[attr-defined] + setattr(record, "request_id", request_id_var.get("")) except Exception: - record.request_id = "" # type: ignore[attr-defined] + setattr(record, "request_id", "") return True class EmojiFormatter(logging.Formatter): """Custom formatter that adds emoji based on log level.""" - def format(self, record): + def format(self, record: logging.LogRecord) -> str: + """Format a log record as a single line with a prefix and emoji.""" # Get emoji for log level emoji = EMOJI_MAP.get(record.levelname, "📂") @@ -96,10 +100,10 @@ def get_logger(name: str, level: Optional[int] = None) -> logging.Logger: return logger # Add SUCCESS level -logging.SUCCESS = 25 # Between INFO (20) and WARNING (30) -logging.addLevelName(logging.SUCCESS, "SUCCESS") +SUCCESS_LEVEL: Final[int] = 25 # Between INFO (20) and WARNING (30) +logging.addLevelName(SUCCESS_LEVEL, "SUCCESS") -def log_success(logger, message: str): +def log_success(logger: logging.Logger, message: str) -> None: """ Log a success message with ✅ emoji. @@ -107,9 +111,9 @@ def log_success(logger, message: str): logger: Logger instance message: Success message """ - logger.log(logging.SUCCESS, message) + logger.log(SUCCESS_LEVEL, message) -def log_structured(logger, level, message: str, **context): +def log_structured(logger: logging.Logger, level: int, message: str, **context: Any) -> None: """Emit a structured JSON log entry with contextual fields.""" payload = { "message": message, diff --git a/shared/result.py b/shared/result.py index a5eb866a..05a827ab 100644 --- a/shared/result.py +++ b/shared/result.py @@ -4,9 +4,12 @@ All service methods return Result[T] to avoid HTTP 500 errors. """ from dataclasses import dataclass, field from enum import Enum -from typing import Any, Dict, Generic, Optional, TypeVar +from typing import Any, Callable, Generic, Optional, TypeVar, cast + +from .types import ErrorCode T = TypeVar("T") +U = TypeVar("U") @dataclass class Result(Generic[T]): @@ -23,33 +26,31 @@ class Result(Generic[T]): data: Optional[T] = None error: Optional[str] = None code: str = "OK" # OK, DEGRADED, TOOL_MISSING, DB_ERROR, UNSUPPORTED, etc. - meta: Dict[str, Any] = field(default_factory=dict) + meta: dict[str, Any] = field(default_factory=dict) @staticmethod - def Ok(data: T, **meta) -> "Result[T]": + def Ok(data: T, **meta: Any) -> "Result[T]": """Create a successful result with data and optional metadata.""" return Result(ok=True, data=data, code="OK", meta=meta) @staticmethod - def Err(code: Any, error: str, **meta) -> "Result[T]": + def Err(code: ErrorCode | str | Enum, error: str, **meta: Any) -> "Result[T]": """Create an error result with code, message, and optional metadata.""" - code_value = code try: - if isinstance(code, Enum): - code_value = code.value + code_value = code.value if isinstance(code, Enum) else code except Exception: code_value = code return Result(ok=False, error=error, code=str(code_value), meta=meta) - def map(self, fn): + def map(self, fn: Callable[[T], U]) -> "Result[U]": """Map the data if ok, otherwise return self.""" if self.ok and self.data is not None: return Result.Ok(fn(self.data), **self.meta) - return self + return cast(Result[U], self) def unwrap(self) -> T: """Get data or raise ValueError if error.""" - if self.ok: + if self.ok and self.data is not None: return self.data raise ValueError(f"[{self.code}] {self.error}") diff --git a/shared/time.py b/shared/time.py index 543d3615..022df2f5 100644 --- a/shared/time.py +++ b/shared/time.py @@ -1,9 +1,12 @@ """ Time utilities for timestamps and performance measurement. """ +from __future__ import annotations + +import logging import time -from typing import Optional from contextlib import contextmanager +from typing import Iterator, Optional def now() -> float: """Get current timestamp in seconds (float).""" @@ -28,7 +31,7 @@ def format_timestamp(ts: Optional[float] = None) -> str: return time.strftime("%Y-%m-%dT%H:%M:%S", time.localtime(ts)) @contextmanager -def timer(label: str, logger=None): +def timer(label: str, logger: Optional[logging.Logger] = None) -> Iterator[None]: """ Context manager for timing operations. diff --git a/shared/types.py b/shared/types.py index 4432143c..5c5f8d8a 100644 --- a/shared/types.py +++ b/shared/types.py @@ -2,7 +2,7 @@ Shared types, enums, and constants. """ from enum import Enum -from typing import Literal +from typing import Final, Literal # File type classifications FileKind = Literal["image", "video", "audio", "model3d", "unknown"] @@ -56,11 +56,12 @@ class ErrorCode(str, Enum): PARSE_ERROR = "PARSE_ERROR" # File extensions by type -EXTENSIONS = { +EXTENSIONS: Final[dict[FileKind, set[str]]] = { "image": {".png", ".jpg", ".jpeg", ".webp", ".gif"}, "video": {".mp4", ".mov", ".webm", ".mkv"}, "audio": {".wav", ".mp3", ".flac", ".ogg", ".m4a", ".aac"}, "model3d": {".obj", ".fbx", ".glb", ".gltf", ".stl"}, + "unknown": set(), } def classify_file(filename: str) -> FileKind: diff --git a/tests/test_filesystem_kickoff_background_scan_race.py b/tests/test_filesystem_kickoff_background_scan_race.py new file mode 100644 index 00000000..e8f7c693 --- /dev/null +++ b/tests/test_filesystem_kickoff_background_scan_race.py @@ -0,0 +1,52 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from backend.routes.handlers import filesystem + + +class _AliveThread: + def is_alive(self) -> bool: + return True + + +class _RaisingPending: + def __init__(self) -> None: + self._size = 0 + + def __setitem__(self, key: str, value: Any) -> None: + raise RuntimeError("boom") + + def move_to_end(self, key: str) -> None: + raise AssertionError("should not be called if __setitem__ fails") + + def popitem(self, *, last: bool) -> tuple[str, Any]: + raise AssertionError("not used in this test") + + def __len__(self) -> int: # pragma: no cover + return self._size + + +def test_kickoff_background_scan_does_not_update_throttle_marker_if_enqueue_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Avoid starting a real background thread during this unit test. + monkeypatch.setattr(filesystem, "_SCAN_WORKER", _AliveThread()) + + # Force the enqueue path to fail deterministically. + monkeypatch.setattr(filesystem, "_SCAN_PENDING", _RaisingPending()) + + filesystem._BACKGROUND_SCAN_LAST.clear() + + filesystem._kickoff_background_scan( + "C:/tmp", + source="output", + root_id=None, + min_interval_seconds=9999.0, + ) + + key = "output||C:/tmp" + assert key not in filesystem._BACKGROUND_SCAN_LAST + diff --git a/tests/test_guess_content_type_for_file.py b/tests/test_guess_content_type_for_file.py new file mode 100644 index 00000000..89846706 --- /dev/null +++ b/tests/test_guess_content_type_for_file.py @@ -0,0 +1,13 @@ +from __future__ import annotations + +from pathlib import Path + +from backend.routes.core.paths import _guess_content_type_for_file + + +def test_guess_content_type_has_explicit_modern_media_mappings() -> None: + assert _guess_content_type_for_file(Path("x.webp")) == "image/webp" + assert _guess_content_type_for_file(Path("x.gif")) == "image/gif" + assert _guess_content_type_for_file(Path("x.webm")) == "video/webm" + assert _guess_content_type_for_file(Path("x.avif")) == "image/avif" +