diff --git a/.gitignore b/.gitignore index 57731696..0f8fd356 100644 --- a/.gitignore +++ b/.gitignore @@ -214,3 +214,7 @@ coverage/ # Old library files a Move library renamed (never deleted; #249 S6b) *.moved-to-* + +# Per-install key behind the workspace scopes and cursors (#430) +workspace-scope.key +.workspace-scope.key.*.tmp diff --git a/TESTING.md b/TESTING.md index 9a198fc2..4390837e 100644 --- a/TESTING.md +++ b/TESTING.md @@ -551,6 +551,49 @@ hold a walk open with a gated scan instead of sleeping. time to network idle and the 10 slowest requests, from a fresh browser context each run. +## Open pages across restarts and source changes (#430) + +A page addresses a listed file as `{root_index, root_scope, path}`. The root +scope is an HMAC over the root's index and canonical path, so the server refuses +(409 `source-changed`) an action aimed at a root that was renumbered or replaced +since the page listed it (a same-named `ComfyUI_00001_.png` sits in every output +folder). Its key, and the listing cursors' key, derive from one random secret in +`workspace-scope.key` beside the database (`utils/workspace_keys.py`; owner-only, +64 hex characters; never in config.json or the database, so no backup or export +carries it; Move library copies it, best effort). A ComfyUI restart therefore keeps +every scope and cursor an open page holds: `tests/test_api_workspace_restart.py` +proves it with a second API on its own server and with two fresh interpreters on +the same database. The key file is created with a temp file and `os.link` (of two +first starts one file wins; without hard links `O_EXCL`), re-read after any write, +replaced with a WARNING when corrupt, and a folder that cannot hold it gives that +process a per-process key (one WARNING; open pages then refresh after a restart). +Tests never write it outside their own temp folders. + +The cursor is `{"v": 2, "key"}` with the listing scope (it hashes every absolute +root) bound into its MAC only, never sent. Bump `v` whenever the key tuple's shape +changes: a cursor now outlives a restart and an upgrade. A cursor the server +cannot verify (another key, root list or filter set) answers 400 +`cursor-stale`; Images then shows "The image collection changed. Refresh image +collection to continue." and its Retry becomes **Refresh image collection**. + +When a source really changed, `POST /prompt_manager/images/rescope` (`{"target": +{...}}`, validated by `media_target`, index ≤ 4095) maps the held target to the +first current root whose scope at the old index equals the held scope, and answers +the file's current workspace descriptor; a removed or replaced root, another key +or a foreign scope is 409 `source-changed`, a missing file 404 `file-missing`, a +path leaving the root or a preview 403 (always `no-store`). Only the Images roots +are mapped. Images (`images-source-recovery.mjs`) asks once per held +`(key, scope)` pair, shared by the inspector (an inspection that turns +`refreshRequired` with `code: "source-changed"`), the viewer (a media load error) +and the Tag image dialog. Found under another index, nothing is kept by an old key +(`root_index:path` could name another source's file): the viewer closes, the checks +clear, a Source filter on the old index follows the file (another one falls back to +All sources), only that file stays selected and the collection lists again; a +manual Refresh forgets every pair. Not found, the existing visible Refresh stays. +Browser tests: `workspace-stale-source.spec.mjs` (`/__test__/sources/reverse` and +`/__test__/scope-key/rotate`, both undone by every reset); the manual-Refresh +recovery specs in `workspace-images.spec.mjs` answer every rescope 409. + ## Windows portability preflight Before changing paths, SQLite, streams, restore or cleanup: @@ -1697,10 +1740,11 @@ wins), and toasts the mock's copy verbatim, " tags saved for ." eve for one tag, plus " Prompt association still awaits review." from Needs review. Tagging one file never changes another file (an equal-byte copy included), a prompt, a pair or a link. States the mock lacks (generating, busy, conflict, -changed storage or file, expired capture, unconfirmed save, unavailable model, -video) are text in `#tag-generation-status` only; **Retry** (D116) leads the -actions while the controller allows it, where the other workspace dialogs place -their Retry. The Tags +unavailable model, video) are text in `#tag-generation-status` only; a state that +blocks Save (a failed or refused capture, a changed source, file or storage, an +expired capture, an unconfirmed save) shows its reason beside the actions instead +(D147); **Retry** (D116) leads the action buttons while the controller allows it, +where the other workspace dialogs place their Retry. The Tags page re-reads its lists after every confirmed save, so the Images counts follow. The image-tagging plan's deviation numbers were renumbered by +23 because the @@ -1744,8 +1788,8 @@ Inspect plan keeps D60–D66 (plan D60 → D83, D61 → D84, D65 → D88, D68 its "Auto-tag images" always opens the dialog. Production opens it for the image selected in Images when it was last left; with none it is `aria-disabled` and pressing it toasts "Select an image in Images first." (no new chrome). -- **D116** (ruling T5): **Retry** leads `.dialog-actions` (before Cancel and the - primary) while a failed observation, a failed Generate, an unconfirmed save or an +- **D116** (ruling T5): **Retry** leads the buttons of `.dialog-actions` (before + Cancel and the primary; D147's notice row sits above them) while a failed observation, a failed Generate, an unconfirmed save or an answer that needs a fresh observation (changed storage or file, expired capture) can be retried; the concept has no failure states, so it has no Retry. After a failed Generate it takes Generate's own path (an invalid field reports its @@ -1756,6 +1800,25 @@ Inspect plan keeps D60–D66 (plan D60 → D83, D61 → D84, D65 → D88, D68 Browser tests: "D116: Retry leads the actions after an unconfirmed save…", "D116: Retry after an expired capture observes again and keeps the typed draft (M5)" and the paired `production-unknown-outcome` capture. +- **D147** (#430, beyond the Global Constraints' "status text only"): a state + that blocks Save shows its reason in `div.tag-action-notice` (its sentence in a + `role="status"` span, written only when it changes), the first, full-width row of + the sticky `.dialog-actions`, above the buttons; `#tag-generation-status` is then + empty, so the sentence shows once. Generate and Save are `aria-disabled` and look + it (`.tagging-modal [aria-disabled="true"]`: opacity .55, `not-allowed`). A failed + or refused capture replaces the empty preview with a same-size + `div.tag-image-preview.tag-preview-empty` "Preview unavailable" (`role="img"`). + A changed source (409 `source-changed` from observe, Generate or Save, phase + `source-changed`, never retried as is) first asks the page where the file is + (`onSourceChanged`, see "Open pages across restarts and source changes"): found, + the dialog stays open and observes it there (`controller.retarget`, the typed + draft kept, the old capture released; refused while a save is in flight); not + found, the notice shows the server's sentence and **Refresh image collection** + (Images and Library-embedded galleries; **Reload page** elsewhere), which closes + the dialog first. Opened, generated and empty states are unchanged, so the concept + comparisons are not. Browser tests: `tests/browser/workspace-stale-source.spec.mjs`; + the file-changed and unconfirmed-save checks in `workspace-tagging.spec.mjs` and + `tagging-concept.spec.mjs` read the notice. - **D29** (no prototype copy) in the dialog: no "Prototype · suggestions are simulated…" note, the status reads "Suggestions from . Approve tags to save." (not "Sample suggestions from …"), and the toast has no " in this tab". @@ -1808,7 +1871,8 @@ production-versus-concept difference of PR C and its record: | Image tags first in the inspector and viewer chip row | D88 (ruling T7/C2) | | Auto-tag images `aria-disabled` without a selected image | D115 | | Retry leads the actions when the controller allows it | D116 (ruling T5) | -| States the concept lacks (busy, conflict, changed storage or file, unconfirmed save, unavailable model, video) | Global Constraints: `#tag-generation-status` text only | +| States the concept lacks (busy, conflict, unavailable model, video) | Global Constraints: `#tag-generation-status` text only | +| A state that blocks Save (failed capture, changed source, file or storage, unconfirmed save): its reason and Refresh beside the actions, a "Preview unavailable" box | D147 (#430) | ### Bulk tagging from the selection bar (#249 PR D) diff --git a/database/library_move.py b/database/library_move.py index 1cec52e6..91dc7274 100644 --- a/database/library_move.py +++ b/database/library_move.py @@ -56,9 +56,11 @@ from .models import BUSY_TIMEOUT_SECONDS, PromptModel try: from ..utils import library_location as ll from ..utils.logging_config import get_logger + from ..utils.workspace_keys import KEY_FILE_NAME except ImportError: from utils import library_location as ll from utils.logging_config import get_logger + from utils.workspace_keys import KEY_FILE_NAME logger = get_logger("prompt_manager.database.library_move") @@ -491,7 +493,8 @@ class _Move: sweep_move_temps(target) if ll.read_marker(target).get("state") == ll.MARKER_PENDING: # An earlier move stopped before it switched: its files are ours. - for name in (ll.DATABASE_NAME, ll.CONFIG_NAME) + self.plan.backups: + names = (ll.DATABASE_NAME, ll.CONFIG_NAME, KEY_FILE_NAME) + for name in names + self.plan.backups: _remove(os.path.join(target, name)) for suffix in ("-wal", "-shm", "-journal"): _remove(os.path.join(target, ll.DATABASE_NAME + suffix)) @@ -536,6 +539,7 @@ class _Move: raise MoveFailed("copy-failed") from error _check_copies([(self.plan.config_path, temp, expected)]) copies = copies + [(self.plan.config_path, temp, expected, ll.CONFIG_NAME)] + copies = copies + self._key_copy() self.marker = write_marker( target, ll.MARKER_PENDING, self.plan.db_path, self.plan.config_path ) @@ -556,6 +560,28 @@ class _Move: self.checkpoint("marked") return files + def _key_copy(self) -> list: + """The workspace key's verified copy (#430), or none: it is re-creatable. + + Without it the moved library starts with a new key, so pages left open + across the restart refresh once; that never fails the move. + """ + source = os.path.join(self.plan.source_dir, KEY_FILE_NAME) + if not os.path.isfile(source): + return [] + temp = self.temp(".key") + try: + expected = _hash_copy(source, temp) + _check_copies([(source, temp, expected)]) + except (OSError, MoveFailed) as error: + _remove(temp) + logger.warning( + f"Library move: the workspace key was not copied ({error}); " + "open workspace pages refresh once after the restart" + ) + return [] + return [(source, temp, expected, KEY_FILE_NAME)] + def cleanup(self) -> None: if self.moved: return diff --git a/py/api/__init__.py b/py/api/__init__.py index 76fd0bb1..a259b574 100644 --- a/py/api/__init__.py +++ b/py/api/__init__.py @@ -74,6 +74,7 @@ from .autotag_routes import AutotagRoutesMixin from .lora_integration import LoraIntegrationMixin from .lora_import_routes import LoraImportRoutesMixin from .image_categories import ImageCategoryRoutesMixin +from .source_rescope import SourceRescopeMixin try: from ...database.operations import PromptDatabase @@ -340,6 +341,7 @@ class PromptManagerAPI( LoraIntegrationMixin, LoraImportRoutesMixin, ImageCategoryRoutesMixin, + SourceRescopeMixin, ): """REST API handler for PromptManager operations and web interface. @@ -706,6 +708,7 @@ class PromptManagerAPI( self._register_lora_routes(routes) self._register_lora_import_routes(routes) self._register_image_category_routes(routes) + self._register_rescope_routes(routes) # Register gzip compression middleware (once) global _gzip_registered diff --git a/py/api/images.py b/py/api/images.py index 614287b9..4726d995 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -13,6 +13,7 @@ import re import stat import subprocess # nosec B404 - ffmpeg frames through run_encoder, fixed argv import sys +import threading import time as _time import urllib.parse from bisect import bisect_left, bisect_right, insort @@ -81,6 +82,7 @@ try: reveal_executor, reveal_in_file_manager, ) + from ...utils.workspace_keys import gallery_root_scope, workspace_keys_for except ImportError: from database.image_collections import CollectionMissing from database.image_favorites import MAX_KEY_LENGTH @@ -105,6 +107,7 @@ except ImportError: reveal_executor, reveal_in_file_manager, ) + from utils.workspace_keys import gallery_root_scope, workspace_keys_for IMAGE_EXTENSIONS = _IMAGE_EXTENSIONS @@ -518,7 +521,8 @@ MAX_JSON_BODY_BYTES = 1024 * 1024 _READ_CHUNK = 64 * 1024 -_WORKSPACE_CURSOR_SECRET = os.urandom(32) +# Guards the first load of an API's workspace keys (#430, _workspace_keys). +_WORKSPACE_KEYS_LOCK = threading.Lock() def _workspace_root(query: dict, roots: list[Path]) -> int | None: @@ -618,15 +622,37 @@ def _rated_key(entry: _Listed, rank: int) -> tuple: return (-rank, *_workspace_key(entry, "rating")) -def _workspace_cursor(key: tuple, scope: str) -> str: +# Bump whenever the cursor's key tuple changes shape: a cursor now outlives a +# restart and an upgrade (#430), so an old shape must be refused, not parsed. +_CURSOR_VERSION = 2 +_CURSOR_STALE = "The image collection changed; refresh the gallery" + + +class CursorStale(ValueError): + """A well-formed cursor this listing cannot verify (#430). + + Another key, another root list or other filters: the page refreshes its + collection instead of retrying the same cursor. + """ + + +def _cursor_mac(secret: bytes, scope: str, payload: bytes) -> bytes: + # The listing scope (it hashes every absolute root) is bound in the MAC + # only, never sent: the scope is 64 hex, so the "\n" join is unambiguous. + return hmac.new(secret, scope.encode() + b"\n" + payload, hashlib.sha256).digest() + + +def _workspace_cursor(key: tuple, scope: str, secret: bytes) -> str: + """The signed keyset cursor of *key*; *secret* is the API's cursor key.""" payload = json.dumps( - {"v": 1, "scope": scope, "key": key}, separators=(",", ":"), ensure_ascii=False + {"v": _CURSOR_VERSION, "key": key}, separators=(",", ":"), ensure_ascii=False ).encode() - signature = hmac.new(_WORKSPACE_CURSOR_SECRET, payload, hashlib.sha256).digest() + signature = _cursor_mac(secret, scope, payload) return base64.urlsafe_b64encode(signature + payload).decode().rstrip("=") -def _workspace_cursor_key(token: str, scope: str, order: str) -> tuple: +def _workspace_cursor_key(token: str, scope: str, order: str, secret: bytes) -> tuple: + """The keyset key of *token*; CursorStale when it is not this listing's.""" try: if len(token) > 4096: raise ValueError() @@ -634,13 +660,14 @@ def _workspace_cursor_key(token: str, scope: str, order: str) -> tuple: token + "=" * (-len(token) % 4), altchars=b"-_", validate=True ) signature, payload = raw[:32], raw[32:] - expected = hmac.new(_WORKSPACE_CURSOR_SECRET, payload, hashlib.sha256).digest() - if not hmac.compare_digest(signature, expected): + if len(signature) != 32 or not payload: raise ValueError() + if not hmac.compare_digest(signature, _cursor_mac(secret, scope, payload)): + raise CursorStale(_CURSOR_STALE) data = json.loads(payload) key = data["key"] - if data["v"] != 1 or data["scope"] != scope or not isinstance(key, list): - raise ValueError() + if data["v"] != _CURSOR_VERSION or not isinstance(key, list): + raise CursorStale(_CURSOR_STALE) types = ( (str, str, int, (float, int)) if order in ("name", "name-desc") @@ -654,6 +681,8 @@ def _workspace_cursor_key(token: str, scope: str, order: str) -> tuple: ): raise ValueError() return tuple(key) + except CursorStale: + raise except (ValueError, KeyError, TypeError, UnicodeError, binascii.Error): raise ValueError("Invalid gallery cursor; refresh the gallery") from None @@ -1338,13 +1367,27 @@ def _in_subfolder(rel_dir, subfolder): class ImageRoutesMixin: """Mixin providing image and gallery-related API endpoints.""" + def _workspace_keys(self): + """This API's scope and cursor keys, loaded once (#430). + + They derive from the secret persisted beside the database, so a restart + keeps every scope and cursor an open page holds. The first call reads + (or writes) one small file; later calls return the cached keys, which + a live library move or restore does not change before a restart. + """ + keys = self.__dict__.get("_workspace_keys_cache") + if keys is None: + with _WORKSPACE_KEYS_LOCK: + keys = self.__dict__.get("_workspace_keys_cache") + if keys is None: + db = getattr(self, "db", None) + model = getattr(db, "model", None) + keys = workspace_keys_for(getattr(model, "db_path", None)) + self.__dict__["_workspace_keys_cache"] = keys + return keys + def _gallery_root_scope(self, root: Path, index: int) -> str: - payload = json.dumps( - ["prompt-manager-gallery-root-v1", index, str(_canonical(root))], - separators=(",", ":"), - ensure_ascii=False, - ).encode("utf-8") - return hmac.new(_WORKSPACE_CURSOR_SECRET, payload, hashlib.sha256).hexdigest() + return gallery_root_scope(self._workspace_keys(), str(_canonical(root)), index) def _gallery_scope_error( self, query: Mapping, roots: list[Path] @@ -2008,6 +2051,8 @@ class ImageRoutesMixin: limit, _offset = parse_page_params(request.query, default_limit=100) except ValueError: return bad_request("limit and offset must be integers") + # #430: the first use may create the key file; never on the event loop. + cursor_secret = (await self._run_in_executor(self._workspace_keys)).cursor try: root_index = _workspace_root(request.query, output_dirs) filters = _workspace_filters(request.query) @@ -2031,7 +2076,15 @@ class ImageRoutesMixin: ).hexdigest() token = request.query.get("cursor") after = ( - _workspace_cursor_key(token, scope, filters["order"]) if token else None + _workspace_cursor_key(token, scope, filters["order"], cursor_secret) + if token + else None + ) + except CursorStale as exc: + return web.json_response( + {"success": False, "code": "cursor-stale", "error": str(exc)}, + status=400, + headers={"Cache-Control": "no-store"}, ) except ValueError as exc: return bad_request(str(exc)) @@ -2472,7 +2525,11 @@ class ImageRoutesMixin: positive_count = _COUNT_OMITTED has_more = len(page) > limit page = page[:limit] - next_cursor = _workspace_cursor(key(page[-1]), scope) if has_more else None + next_cursor = ( + _workspace_cursor(key(page[-1]), scope, self._workspace_keys().cursor) + if has_more + else None + ) return ( [(entry.path, entry.mtime, entry.root, entry.index) for entry in page], total, diff --git a/py/api/source_rescope.py b/py/api/source_rescope.py new file mode 100644 index 00000000..caa9bfa9 --- /dev/null +++ b/py/api/source_rescope.py @@ -0,0 +1,100 @@ +"""Where a stale Images target is now (#430): ``POST /prompt_manager/images/rescope``. + +A page addresses a listed file as ``{root_index, root_scope, path}``. When the +sources are renumbered its scope no longer names the root at that index, and +every per-file action answers 409 ``source-changed``. Before it gives up, the +page asks here. The target is mapped only when its scope is, at its old +index, the scope of one of the current roots: the server issued that scope +for exactly that folder, so the answer (the file's current listing +descriptor) names nothing the page could not already address. A removed or +replaced root, a scope of another key and a foreign scope stay refused. + +Only the Images roots (``_get_all_output_dirs``) are mapped; a LoRA-linked +root's target is never re-scoped here. +""" + +import hmac +from pathlib import Path + +from aiohttp import web + +from .images import ( + _forbidden, + _is_thumbnail_cache_path, + _json_body_or_error, + _media_access_error, +) +from .media_target import media_target +from .prompts import bad_request + +# Far above any real source count; keeps the index out of huge-int territory. +MAX_ROOT_INDEX = 4095 +SOURCE_GONE = ( + "The source of this image changed or was removed. Refresh the image " + "collection to find it again." +) + + +def _coded(status: int, code: str, message: str) -> web.Response: + return web.json_response( + {"success": False, "code": code, "error": message}, status=status + ) + + +def _no_store(response: web.Response) -> web.Response: + response.headers["Cache-Control"] = "no-store" + return response + + +class SourceRescopeMixin: + """The rescope route; composed into ``PromptManagerAPI``.""" + + def _register_rescope_routes(self, routes): + @routes.post("/prompt_manager/images/rescope") + async def rescope_route(request): + return _no_store(await self.rescope_image(request)) + + def _current_root_for(self, scope: str, old_index: int, roots: list) -> int | None: + """The index of the first current root *scope* was issued for at *old_index*.""" + for index, root in enumerate(roots): + if hmac.compare_digest(scope, self._gallery_root_scope(root, old_index)): + return index + return None + + def _rescope_blocking(self, scope: str, old_index: int, relative: str): + """The answer to one rescope (blocking: it resolves paths).""" + roots = list(self._get_all_output_dirs()) + index = self._current_root_for(scope, old_index, roots) + if index is None: + return _coded(409, "source-changed", SOURCE_GONE) + root = Path(roots[index]) + path = root / relative + denied = _media_access_error(path, [root]) + if denied is not None: + return denied + try: + mtime = path.stat().st_mtime if path.is_file() else None + except OSError: + mtime = None + if mtime is None: + return _coded(404, "file-missing", "Image file not found") + [image] = self._format_workspace_page([(path, mtime, root, index)]) + return web.json_response({"success": True, "image": image}) + + async def rescope_image(self, request) -> web.Response: + data, error = await _json_body_or_error(request) + if error is not None: + return error + if set(data) != {"target"}: + return bad_request("Send exactly one target") + target, error = media_target(data["target"]) + if error is not None: + return error + old_index, scope, relative = target + if old_index > MAX_ROOT_INDEX: + return bad_request("Invalid gallery root") + if _is_thumbnail_cache_path(relative): + return _forbidden("Previews are not re-scoped; use the original") + return await self._run_in_executor( + self._rescope_blocking, scope, old_index, relative + ) diff --git a/tests/browser/prompt-inspector.spec.mjs b/tests/browser/prompt-inspector.spec.mjs index 8841598a..7abdabb5 100644 --- a/tests/browser/prompt-inspector.spec.mjs +++ b/tests/browser/prompt-inspector.spec.mjs @@ -38,6 +38,9 @@ async function chooseDestination(page) { test('collection viewer rejects stale source and sends only refreshed API evidence',async({page,request,context})=>{ const host=await fakeHost(context,page); + // #430: the file is never found again here, so the manual Refresh is what this covers (the found-again path is + // workspace-stale-source.spec.mjs). + await page.route('**/prompt_manager/images/rescope',route=>route.fulfill({status:409,json:{success:false,code:'source-changed',error:'The source of this image changed or was removed.'}})); await page.addInitScript(()=>{window.copied=[];Object.defineProperty(navigator,'clipboard',{configurable:true,value:{writeText:async text=>window.copied.push(text)}});}); try { const loaded=await(await request.get('/prompt_manager/images/output?mode=workspace&limit=40')).json();const stale=loaded.images.find(image=>image.root_index===0&&image.path==='shared/same.png'); diff --git a/tests/browser/server.py b/tests/browser/server.py index dab18015..e2dd9798 100644 --- a/tests/browser/server.py +++ b/tests/browser/server.py @@ -46,6 +46,7 @@ from tests.browser.workspace_library_editor_fixture import ( ) from tests.browser.workspace_vocabulary_fixture import register_vocabulary_commands from tests.browser.workspace_tagging_fixture import register_tagging_commands +from tests.browser.source_change_fixture import register_source_change_commands from tests.browser.workspace_review_fixture import register_review_commands from tests.browser.workspace_diagnostics_fixture import ( register_diagnostics_commands, @@ -225,7 +226,8 @@ def create_app(root: Path) -> web.Application: history_commands = register_workspace_history_commands(routes, api, root) library_editor_commands = register_library_editor_commands(routes, api, root) register_vocabulary_commands(routes, api, root) - register_tagging_commands(routes, api, root, model) + restore_sources = register_source_change_commands(routes, api) + register_tagging_commands(routes, api, root, model, on_reset=restore_sources) register_review_commands(routes, api, root) reveal_stub = register_reveal_stub(routes, api, root) register_activity_commands(app, routes, api) @@ -432,6 +434,7 @@ def create_app(root: Path) -> web.Application: async def reset(request: web.Request) -> web.Response: nonlocal history_fixture request_holds.release() + restore_sources() # No preview queued by the last test may land in the new fixture. await api._run_in_executor(api.close_auto_thumbnails) library_editor_commands.reset() diff --git a/tests/browser/source_change_fixture.py b/tests/browser/source_change_fixture.py new file mode 100644 index 00000000..4400404f --- /dev/null +++ b/tests/browser/source_change_fixture.py @@ -0,0 +1,66 @@ +"""Source changes under an open workspace page (#430), undone by every reset. + +- ``POST /__test__/sources/reverse``: the image sources are listed in reverse + order, as System › Sources would renumber them, so every held + ``root_scope`` names another index and the page must re-scope its file. +- ``POST /__test__/scope-key/rotate``: the API's scope and cursor keys + change, as when ``workspace-scope.key`` is lost before a restart: a held + scope then cannot be mapped and the page offers Refresh. +""" + +from __future__ import annotations + +import secrets +from typing import TYPE_CHECKING, Callable + +from aiohttp import web + +from utils.workspace_keys import WorkspaceKeys + +if TYPE_CHECKING: + from py.api import PromptManagerAPI + +_CACHE = "_workspace_keys_cache" + + +def register_source_change_commands( + routes: web.RouteTableDef, api: PromptManagerAPI +) -> Callable[[], None]: + """Register the commands; returns the restore every reset runs.""" + # Imported here: py.config needs the host stubs create_app installs first. + from py.config import GalleryConfig + + saved: dict = {} + + def restore() -> None: + if "roots" in saved: + GalleryConfig.MONITORING_DIRECTORIES = saved.pop("roots") + api.invalidate_gallery_cache(retire=True) + if "keys" in saved: + keys = saved.pop("keys") + if keys is None: + api.__dict__.pop(_CACHE, None) + else: + api.__dict__[_CACHE] = keys + + @routes.post("/__test__/sources/reverse") + async def reverse(request: web.Request) -> web.Response: + if request.query or request.can_read_body: + raise web.HTTPBadRequest() + roots = list(GalleryConfig.MONITORING_DIRECTORIES) + saved.setdefault("roots", roots) + GalleryConfig.MONITORING_DIRECTORIES = roots[::-1] + await api._run_in_executor(api.invalidate_gallery_cache, True) + return web.json_response({"success": True}) + + @routes.post("/__test__/scope-key/rotate") + async def rotate(request: web.Request) -> web.Response: + if request.query or request.can_read_body: + raise web.HTTPBadRequest() + saved.setdefault("keys", api.__dict__.get(_CACHE)) + api.__dict__[_CACHE] = WorkspaceKeys( + secrets.token_bytes(32), secrets.token_bytes(32), False + ) + return web.json_response({"success": True}) + + return restore diff --git a/tests/browser/tagging-concept.spec.mjs b/tests/browser/tagging-concept.spec.mjs index 756cde9c..4d7eec6a 100644 --- a/tests/browser/tagging-concept.spec.mjs +++ b/tests/browser/tagging-concept.spec.mjs @@ -231,6 +231,8 @@ test('paired Tag dialog captures', async ({ page, request }, testInfo) => { await expect(await request.post('/__test__/tagging/suggestions', { data: { scored: ALPINE } })).toBeOK(); const save = () => tagDialog(page).locator('.dialog-actions .ca-primary').click(); const status = () => tagDialog(page).locator('#tag-generation-status'); + // D147: a state that blocks Save shows its reason beside the actions, not in the status line. + const notice = () => tagDialog(page).locator('.dialog-actions > .tag-action-notice'); // Unavailable model. await expect(await request.post('/__test__/tagging/models', { data: { unavailable: ['wd14-swinv2'] } })).toBeOK(); @@ -269,7 +271,8 @@ test('paired Tag dialog captures', async ({ page, request }, testInfo) => { await saveImageTags(request, path, CONCEPT_TAGS); await openTagDialog(page, 'production', 'dark'); await save(); - await expect(status()).not.toHaveText(''); + await expect(notice()).toHaveText('The file changed since it was previewed. Reopen the Tag dialog.'); + await expect(status()).toHaveText(''); await shot('production-file-changed'); await page.unroute(writes); await page.keyboard.press('Escape'); @@ -278,7 +281,7 @@ test('paired Tag dialog captures', async ({ page, request }, testInfo) => { await page.route(writes, route => route.fulfill({ status: 504, body: 'Gateway Timeout' })); await openTagDialog(page, 'production', 'dark'); await save(); - await expect(status()).toHaveText('The save could not be confirmed. Retry sends the same tags again.'); + await expect(notice()).toHaveText('The save could not be confirmed. Retry sends the same tags again.'); expect((await inventory(page)).actions.buttons.map(button => [button.text, button.primary, button.disabled])).toEqual( [['Retry', false, false], ['Cancel', false, false], ['Save manual tags', true, true]]); await shot('production-unknown-outcome'); diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 5c2184b4..78e5baff 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -569,6 +569,10 @@ test('source and folder discovery failure stays visible and Import Rescan restor async function staleSourceRecovery(page, request, roots) { await expect(await request.post('/__test__/reset', { data: { extra_images: 80 } })).toBeOK(); await page.setViewportSize({ width: 1600, height: 1100 }); + // #430: the page first asks the server where a stale file is now. These tests cover the manual Refresh, so the + // file is never found; workspace-stale-source.spec.mjs covers a renumbered file that is found again. + await page.route('**/prompt_manager/images/rescope', route => route.fulfill({ status: 409, + json: { success: false, code: 'source-changed', error: 'The source of this image changed or was removed.' } })); await page.goto(workspace); const source = refineDialog(page).getByLabel('Source', { exact: true }); const folder = refineDialog(page).getByLabel('Folder', { exact: true }); diff --git a/tests/browser/workspace-stale-source.spec.mjs b/tests/browser/workspace-stale-source.spec.mjs new file mode 100644 index 00000000..16af0827 --- /dev/null +++ b/tests/browser/workspace-stale-source.spec.mjs @@ -0,0 +1,208 @@ +// #430: an open workspace page whose sources changed under it. A ComfyUI restart no longer changes any scope (the +// key is persisted, see tests/test_api_workspace_restart.py); what is left is a real source change. Renumbered +// sources (`/__test__/sources/reverse`) are found again: the Tag image dialog stays open on its file and works, and +// the panel keeps the file. A scope that cannot be mapped (`/__test__/scope-key/rotate`, a lost key) blocks Generate +// and Save visibly, says why beside them and offers Refresh. Both commands are undone by every reset. +import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; +import { test, expect } from './fixtures.mjs'; + +const images = '/prompt_manager/workspace#/images'; +const FIRST = 'tagging/dunes_00001.png', SECOND = 'tagging/dunes_00002.png'; +const SOURCE_CHANGED = 'The source changed. Refresh the image collection before tagging this file.'; +const card = (page, path, root = 0) => page.locator(`article[data-image-key="${root}:${path}"]`); +const cards = page => page.locator('.gallery-body article[data-image-key]'); +const tagButton = (page, path, root = 0) => card(page, path, root).getByRole('button', { name: `Tag or auto-tag ${path.split('/').pop()}`, exact: true }); +const infoButton = (page, path, root = 0) => card(page, path, root).getByRole('button', { name: `Inspect ${path.split('/').pop()}`, exact: true }); +const dialog = page => page.getByRole('dialog', { name: 'Tag image', exact: true }); +const manual = page => dialog(page).getByRole('textbox', { name: 'Image tags · edit manually', exact: true }); +const generate = page => dialog(page).getByRole('button', { name: 'Generate suggestions', exact: true }); +const primary = page => dialog(page).locator('.dialog-actions .ca-primary'); +const notice = page => dialog(page).locator('.dialog-actions > .tag-action-notice'); +const status = page => dialog(page).locator('#tag-generation-status'); +const chips = page => dialog(page).locator('#tag-suggestions .chip.tag-suggestion'); +const panel = page => page.getByRole('complementary', { name: 'Selected image details', exact: true }); +const toast = page => page.locator('#workspace-toast'); +const answered = (page, suffix, method = 'POST') => page.waitForResponse(item => new URL(item.url()).pathname.endsWith(suffix) && item.request().method() === method); + +test.beforeEach(async ({ page, request, browserName }) => { + await startCoverage(page, browserName); + await expect(await request.post('/__test__/tagging/reset', { data: {} })).toBeOK(); + page.staleErrors = []; + page.on('pageerror', error => page.staleErrors.push(error.message)); +}); +test.afterEach(async ({ page, request, browserName }) => { + await saveCoverage(page, browserName); + expect(page.staleErrors).toEqual([]); + // The next spec file starts on the standard sources and key. + await expect(await request.post('/__test__/tagging/reset', { data: {} })).toBeOK(); +}); + +async function openTag(page, path = FIRST, root = 0) { + const observed = answered(page, '/image-annotations/observe'); + await tagButton(page, path, root).click(); + await expect(dialog(page)).toBeVisible(); + return observed; +} +async function savedTags(request, path, root) { + const listing = await (await request.get('/prompt_manager/images/output?mode=workspace&limit=40')).json(); + const item = listing.images.find(image => image.path === path && image.root_index === root); + const target = { root_index: root, root_scope: item.root_scope, path }; + const observed = await (await request.post('/prompt_manager/image-annotations/observe', { data: { target } })).json(); + await request.delete(`/prompt_manager/image-annotations/capture/${observed.capture.token}`); + return observed.tags; +} + +test('renumbered sources under an open Tag dialog: it finds the file again, keeps the draft and Generate works', async ({ page, request }) => { + await page.goto(images); + await expect(card(page, FIRST)).toHaveClass(/\bselected\b/); + expect((await openTag(page)).status()).toBe(200); + await manual(page).fill('kept_draft'); + await expect(await request.post('/__test__/sources/reverse')).toBeOK(); + + // Generate is refused (the held scope names another index now); the page asks where the file is and retargets. + const refused = answered(page, '/image-annotations/suggest'); + const rescoped = answered(page, '/images/rescope'); + const observedAgain = answered(page, '/image-annotations/observe'); + await generate(page).click(); + expect((await refused).status()).toBe(409); + const mapped = await (await rescoped).json(); + expect([mapped.image.root_index, mapped.image.path]).toEqual([1, FIRST]); + const observed = await observedAgain; + expect(observed.status()).toBe(200); + expect(observed.request().postDataJSON().target).toEqual({ root_index: 1, root_scope: mapped.image.root_scope, path: FIRST }); + await expect(dialog(page)).toBeVisible(); + await expect(notice(page)).toHaveCount(0); + await expect(manual(page)).toHaveValue('kept_draft'); + await expect(dialog(page).locator('img.tag-image-preview')).toHaveAttribute('src', /\/image-annotations\/capture\//); + + // The page listed again under the new numbering and kept the file selected. + await expect(card(page, FIRST, 1)).toHaveClass(/\bselected\b/); + await expect(card(page, FIRST, 0)).toHaveCount(0); + + const suggested = answered(page, '/image-annotations/suggest'); + await generate(page).click(); + expect((await suggested).status()).toBe(200); + await expect(chips(page).first()).toBeVisible(); + const saved = answered(page, '/image-annotations/tags', 'PUT'); + await primary(page).click(); + expect((await saved).request().postDataJSON().target.root_index).toBe(1); + await expect(toast(page)).toContainText('tags saved for dunes_00001.png.'); + expect(await savedTags(request, FIRST, 1)).toContain('kept_draft'); +}); + +test('the inspector finds a renumbered file again; checked files from the old numbering are cleared', async ({ page, request }) => { + await page.goto(images); + await expect(card(page, FIRST)).toHaveClass(/\bselected\b/); + await card(page, FIRST).getByRole('checkbox', { name: /^Select / }).check(); + await expect(page.locator('.bulkbar')).toBeVisible(); + await expect(await request.post('/__test__/sources/reverse')).toBeOK(); + + const refused = page.waitForResponse(item => new URL(item.url()).pathname === `/prompt_manager/images/inspect/${SECOND}` && + new URL(item.url()).searchParams.get('root') === '0'); + const reread = page.waitForResponse(item => new URL(item.url()).pathname === `/prompt_manager/images/inspect/${SECOND}` && + new URL(item.url()).searchParams.get('root') === '1'); + await infoButton(page, SECOND).click(); + expect((await refused).status()).toBe(409); + expect((await reread).status()).toBe(200); + await expect(card(page, SECOND, 1)).toHaveClass(/\bselected\b/); + await expect(panel(page)).not.toContainText('The source changed'); + await expect(panel(page).getByRole('button', { name: 'Refresh image collection', exact: true })).toHaveCount(0); + // A checked key of the old list could name a same-named file in another source: nothing stays checked. + await expect(page.locator('.bulkbar')).toBeHidden(); + await expect(cards(page).getByRole('checkbox', { checked: true })).toHaveCount(0); +}); + +test('a scope that cannot be mapped: Generate and Save are visibly disabled, the reason and Refresh sit beside them', async ({ page }) => { + await page.goto(images); + await expect(card(page, FIRST)).toBeVisible(); + await expect(await page.request.post('/__test__/scope-key/rotate')).toBeOK(); + const rescoped = answered(page, '/images/rescope'); + expect((await openTag(page)).status()).toBe(409); + expect((await rescoped).status()).toBe(409); + + await expect(notice(page)).toContainText(SOURCE_CHANGED); + await expect(status(page)).toHaveText(''); + const refresh = notice(page).getByRole('button', { name: 'Refresh image collection', exact: true }); + await expect(refresh).toBeVisible(); + for (const button of [generate(page), primary(page)]) { + await expect(button).toHaveAttribute('aria-disabled', 'true'); + await expect(button).toHaveCSS('opacity', '0.55'); + } + // The notice is its own first row of the sticky actions, above the buttons. + const rows = await dialog(page).locator('.dialog-actions').evaluate(actions => { + const noticeBox = actions.firstElementChild.getBoundingClientRect(); + return { first: actions.firstElementChild.className, below: [...actions.querySelectorAll(':scope > button')].every(button => button.getBoundingClientRect().top >= noticeBox.bottom - 1) }; + }); + expect(rows).toEqual({ first: 'tag-action-notice', below: true }); + const preview = dialog(page).locator('.tag-image-preview'); + await expect(preview).toHaveText('Preview unavailable'); + await expect(preview).toHaveAttribute('role', 'img'); + const suggest = []; + page.on('request', item => { if (item.url().endsWith('/image-annotations/suggest')) suggest.push(item.url()); }); + // Pressed anyway (Playwright refuses an aria-disabled target, so the press is dispatched), nothing is sent. + await generate(page).dispatchEvent('click'); + await primary(page).dispatchEvent('click'); + expect(suggest).toEqual([]); + await page.screenshot({ path: test.info().outputPath('stale-source-tag-dialog.png') }); + + // Refresh closes the dialog and lists again with the current scopes; Tag then works. + const relisted = page.waitForResponse(item => item.url().includes('/images/output?')); + await refresh.click(); + await expect(dialog(page)).toHaveCount(0); + await relisted; + await expect(card(page, FIRST)).toBeVisible(); + expect((await openTag(page)).status()).toBe(200); + await expect(notice(page)).toHaveCount(0); + await expect(generate(page)).not.toHaveAttribute('aria-disabled', 'true'); +}); + +test('an unmappable inspector read keeps its visible Refresh, and Refresh recovers the panel', async ({ page }) => { + await page.goto(images); + await expect(card(page, FIRST)).toHaveClass(/\bselected\b/); + await expect(await page.request.post('/__test__/scope-key/rotate')).toBeOK(); + const rescoped = answered(page, '/images/rescope'); + await infoButton(page, SECOND).click(); + expect((await rescoped).status()).toBe(409); + await expect(panel(page)).toContainText('The source changed. Refresh the image collection before inspecting this file.'); + const relisted = page.waitForResponse(item => item.url().includes('/images/output?')); + await panel(page).getByRole('button', { name: 'Refresh image collection', exact: true }).click(); + await relisted; + await expect(cards(page).first()).toHaveClass(/\bselected\b/); + await expect(panel(page)).not.toContainText('The source changed'); +}); + +test('a Load more cursor the server cannot verify offers Refresh instead of a dead Retry', async ({ page, request }) => { + await expect(await request.post('/__test__/tagging/reset', { data: { extra_images: 60 } })).toBeOK(); + await page.goto(images); + await expect(cards(page).first()).toBeVisible(); + await expect(page.locator('#gallery-continuation')).toContainText('more load as you scroll'); + await expect(await request.post('/__test__/scope-key/rotate')).toBeOK(); + const stale = page.waitForResponse(item => item.url().includes('/images/output?') && new URL(item.url()).searchParams.has('cursor')); + await page.locator('#gallery-continuation').scrollIntoViewIfNeeded(); + const answer = await stale; + expect(answer.status()).toBe(400); + expect((await answer.json()).code).toBe('cursor-stale'); + const continuation = page.locator('#gallery-continuation'); + await expect(continuation).toContainText('The image collection changed. Refresh image collection to continue.'); + const relisted = page.waitForResponse(item => item.url().includes('/images/output?') && !new URL(item.url()).searchParams.has('cursor')); + await continuation.getByRole('button', { name: 'Refresh image collection', exact: true }).click(); + expect((await relisted).status()).toBe(200); + await expect(continuation).not.toContainText('The image collection changed'); +}); + +test('a viewer whose media source was renumbered closes onto the file, kept selected', async ({ page, request }) => { + await page.goto(images); + await expect(card(page, FIRST)).toHaveClass(/\bselected\b/); + await expect(await request.post('/__test__/sources/reverse')).toBeOK(); + const rescoped = answered(page, '/images/rescope'); + await card(page, FIRST).locator('.image-open').click(); + const mapped = await (await rescoped).json(); + expect([mapped.image.root_index, mapped.image.path]).toEqual([1, FIRST]); + await expect(page.locator('#viewer-dialog')).toHaveCount(0); + await expect(card(page, FIRST, 1)).toHaveClass(/\bselected\b/); + // Opened again, it shows the file under its current source. + await card(page, FIRST, 1).locator('.image-open').click(); + await expect(page.locator('#viewer-dialog[open]')).toBeVisible(); + await expect(page.locator('#viewer-dialog').getByText('Media could not be loaded. Retry this file.', { exact: true })).toBeHidden(); + await expect.poll(() => page.locator('#viewer-dialog img').first().evaluate(image => image.complete && image.naturalWidth > 0)).toBe(true); +}); diff --git a/tests/browser/workspace-tagging.spec.mjs b/tests/browser/workspace-tagging.spec.mjs index eaefe7ba..cc39bb33 100644 --- a/tests/browser/workspace-tagging.spec.mjs +++ b/tests/browser/workspace-tagging.spec.mjs @@ -21,6 +21,8 @@ const model = page => dialog(page).getByRole('combobox', { name: 'Tagging model' const generate = page => dialog(page).getByRole('button', { name: 'Generate suggestions', exact: true }); const chips = page => dialog(page).locator('#tag-suggestions .chip.tag-suggestion'); const status = page => dialog(page).locator('#tag-generation-status'); +// D147: the reason a state blocks Save, on its own row in the sticky actions. +const notice = page => dialog(page).locator('.dialog-actions > .tag-action-notice'); const toast = page => page.locator('#workspace-toast'); const nav = (page, label) => page.locator('aside.sidebar nav').getByRole('link', { name: new RegExp(`^${label}(?:\\s*[\\d,]+)?$`) }); const FIRST = 'tagging/dunes_00001.png', SECOND = 'tagging/dunes_00002.png', COPY = 'copies/dunes_00001.png'; @@ -716,7 +718,8 @@ test('D116: Retry leads the actions after an unconfirmed save and resends the sa let lost = true; await page.route('**/prompt_manager/image-annotations/tags', route => (lost ? (lost = false, route.fulfill({ status: 504, body: 'Gateway Timeout' })) : route.continue())); await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); - await expect(status(page)).toHaveText('The save could not be confirmed. Retry sends the same tags again.'); + await expect(notice(page)).toHaveText('The save could not be confirmed. Retry sends the same tags again.'); + await expect(status(page)).toHaveText(''); const inventory = await page.locator('dialog#modal').evaluate(taggingInventory); expect(inventory.actions.buttons.map(button => [button.text, button.primary, button.disabled])).toEqual( [['Retry', false, false], ['Cancel', false, false], ['Save manual tags', true, true]]); diff --git a/tests/browser/workspace_tagging_fixture.py b/tests/browser/workspace_tagging_fixture.py index e1250763..61a16177 100644 --- a/tests/browser/workspace_tagging_fixture.py +++ b/tests/browser/workspace_tagging_fixture.py @@ -180,9 +180,16 @@ def tagging_snapshot(api: PromptManagerAPI) -> dict: def register_tagging_commands( - routes: web.RouteTableDef, api: PromptManagerAPI, root: Path, model + routes: web.RouteTableDef, + api: PromptManagerAPI, + root: Path, + model, + on_reset=lambda: None, ) -> None: - """Register fixed seed, snapshot and fake-model commands; no paths or SQL input.""" + """Register fixed seed, snapshot and fake-model commands; no paths or SQL input. + + ``on_reset`` runs first on every tagging reset (the source-change undo). + """ # The fake download (``/__test__/tagging/download``): waits on ``event``, # then answers ``result``. Every reset sets the event and puts back the # model's raising default, so no test can leave a worker hanging. @@ -216,6 +223,7 @@ def register_tagging_commands( raise web.HTTPBadRequest() if not 0 <= extra <= MAX_EXTRA_IMAGES: raise web.HTTPBadRequest() + on_reset() model.unavailable = set() model.suggestions = None model.loaded = None diff --git a/tests/fixtures/legacy_contract_snapshot.json b/tests/fixtures/legacy_contract_snapshot.json index 27568173..62ee7af2 100644 --- a/tests/fixtures/legacy_contract_snapshot.json +++ b/tests/fixtures/legacy_contract_snapshot.json @@ -121,6 +121,7 @@ "POST /prompt_manager/images/generate-thumbnails", "POST /prompt_manager/images/generate-thumbnails/progress", "POST /prompt_manager/images/link", + "POST /prompt_manager/images/rescope", "POST /prompt_manager/images/reveal", "POST /prompt_manager/jobs/autotag", "POST /prompt_manager/jobs/duplicates", diff --git a/tests/js/workspace-image-tagging-state.test.js b/tests/js/workspace-image-tagging-state.test.js index 4f64b71f..4b7ea2ac 100644 --- a/tests/js/workspace-image-tagging-state.test.js +++ b/tests/js/workspace-image-tagging-state.test.js @@ -253,15 +253,16 @@ test('an unreadable or malformed models read falls back to the documented defaul test('observe failures and malformed answers end in observe-error and retry observes again', async () => { const { createImageTaggingController } = await load(); - let answer = () => fail(409, 'source-changed', 'The source changed. Refresh the image collection before tagging this file.'); + let answer = () => fail(500, 'observe-failed'); const h = harness(defaultRoutes({ [`POST ${OBSERVE}`]: () => answer() })); const c = createImageTaggingController({ api: h.api, target: TARGET }); await c.open(); assert.equal(c.state.phase, 'observe-error'); - assert.equal(c.state.statusText, 'The source changed. Refresh the image collection before tagging this file.'); + assert.equal(c.state.statusText, 'The image could not be opened for tagging.'); assert.equal(c.state.canSave, false); assert.equal(c.state.canGenerate, false); assert.equal(c.state.canRetry, true); + assert.equal(c.state.blocked, true, 'a failed capture blocks Generate and Save with its reason'); answer = () => response(200, observed({ tag_revision: -1 })); await c.retry(); assert.equal(c.state.phase, 'observe-error'); @@ -274,9 +275,71 @@ test('observe failures and malformed answers end in observe-error and retry obse await c.retry(); await settle(); assert.equal(c.state.phase, 'ready'); + assert.equal(c.state.blocked, false); assert.equal(h.of('POST', OBSERVE).length, 4); }); +test('#430: a changed source blocks the dialog until it is retargeted, keeping the draft', async () => { + const { createImageTaggingController } = await load(); + const SOURCE = 'The source changed. Refresh the image collection before tagging this file.'; + const fresh = { root_index: 1, root_scope: 'e'.repeat(64), path: TARGET.path }; + const h = harness(defaultRoutes({ + [`POST ${OBSERVE}`]: call => (call.body.target.root_scope === SCOPE ? fail(409, 'source-changed', SOURCE) : response(200, observed())), + })); + const c = createImageTaggingController({ api: h.api, target: TARGET }); + await c.open(); + assert.equal(c.state.phase, 'source-changed'); + assert.equal(c.state.statusText, SOURCE); + assert.deepEqual([c.state.canSave, c.state.canGenerate, c.state.canRetry, c.state.blocked], [false, false, false, true]); + assert.equal(await c.retry(), false, 'the same stale target is never observed again'); + assert.equal(h.of('POST', OBSERVE).length, 1); + assert.throws(() => c.retarget({ ...fresh, root_scope: 'bad' }), TypeError); + await c.retarget(fresh); + await settle(); + assert.equal(c.state.phase, 'ready'); + assert.deepEqual(h.of('POST', OBSERVE).at(-1).body.target, fresh); + assert.deepEqual({ ...c.target }, fresh); + // Generate and Save now carry the new target. + c.setManual('kept draft'); + await c.generate(); + assert.deepEqual(h.of('POST', SUGGEST)[0].body.target, fresh); +}); + +test('#430: a source change answered to Generate or Save also blocks; retarget keeps the typed draft', async () => { + for (const route of [SUGGEST, TAGS]) { + const method = route === TAGS ? 'PUT' : 'POST'; + let stale = true; + const { c, h } = await opened({ + [`${method} ${route}`]: call => (stale ? fail(409, 'source-changed', 'server says source') : + route === TAGS ? response(200, receipt(call.body.tags)) : response(200, suggested())), + }); + c.setManual('kept draft'); + if (route === TAGS) await c.save(); else await c.generate(); + assert.equal(c.state.phase, 'source-changed'); + assert.equal(c.state.statusText, 'server says source'); + stale = false; + await c.retarget({ root_index: 2, root_scope: 'f'.repeat(64), path: TARGET.path }); + await settle(); + assert.equal(c.state.phase, 'ready'); + assert.equal(c.state.manual, 'kept draft'); + assert.equal(h.of('DELETE', CAPTURE).length, 1, 'the old capture is released'); + } +}); + +test('#430: retarget waits for nothing: it refuses while a save is in flight and after close', async () => { + const gate = deferred(); + const { c, h } = await opened({ [`PUT ${TAGS}`]: () => gate.promise }); + const saving = c.save(); + await flush(); + assert.equal(c.state.phase, 'saving'); + assert.equal(await c.retarget({ ...TARGET, root_index: 3 }), false); + gate.resolve(response(200, receipt(['Minimal']))); + await saving; + c.cancel(); + assert.equal(await c.retarget({ ...TARGET, root_index: 3 }), false); + assert.equal(h.of('POST', OBSERVE).length, 1); +}); + test('model change clears suggestions count and status but keeps manual', async () => { const { c } = await opened(); c.setManual('Minimal, dune'); @@ -547,7 +610,7 @@ test('write errors keep the draft and allow another save', async () => { }); test('storage-changed and file-changed disable save until reopen', async () => { - for (const [code, phase] of [['storage-changed', 'storage-changed'], ['file-changed', 'file-changed'], ['source-changed', 'file-changed'], ['capture-expired', 'capture-expired']]) { + for (const [code, phase] of [['storage-changed', 'storage-changed'], ['file-changed', 'file-changed'], ['source-changed', 'source-changed'], ['capture-expired', 'capture-expired']]) { const message = `server says ${code}`; let answer = () => fail(409, code, message); const { c, h } = await opened({ [`PUT ${TAGS}`]: () => answer() }); diff --git a/tests/js/workspace-images-source-recovery.test.js b/tests/js/workspace-images-source-recovery.test.js new file mode 100644 index 00000000..a03a968d --- /dev/null +++ b/tests/js/workspace-images-source-recovery.test.js @@ -0,0 +1,172 @@ +// Run with: node --test tests/js/workspace-images-source-recovery.test.js +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const load = () => import('../../web/js/workspace/images-source-recovery.mjs'); + +const OLD = Object.freeze({ root_index: 0, root_scope: 'a'.repeat(64), path: 'sub/a.png', filename: 'a.png' }); +const FRESH = Object.freeze({ root_index: 1, root_scope: 'b'.repeat(64), path: 'sub/a.png', filename: 'a.png', url: '/u' }); + +function fakeApi(answer) { + const calls = []; + return { calls, post: async (url, body, options) => { calls.push({ url, body, options }); return answer(body); } }; +} +const flush = () => new Promise(resolve => setImmediate(resolve)); + +test('rescopeImage posts only the target and returns the fresh descriptor', async () => { + const { rescopeImage, RESCOPE_URL } = await load(); + const api = fakeApi(() => ({ success: true, image: FRESH })); + const signal = new AbortController().signal; + const fresh = await rescopeImage(api, OLD, { signal }); + assert.deepEqual(fresh, FRESH); + assert.ok(Object.isFrozen(fresh)); + assert.deepEqual(api.calls, [{ url: RESCOPE_URL, body: { target: { root_index: 0, root_scope: OLD.root_scope, path: 'sub/a.png' } }, options: { signal } }]); +}); + +test('rescopeImage answers null for a refusal, a malformed answer, another path or a bad target', async () => { + const { rescopeImage } = await load(); + const refused = { post: async () => { throw Object.assign(new Error('gone'), { status: 409, code: 'source-changed' }); } }; + assert.equal(await rescopeImage(refused, OLD), null); + const lost = { post: async () => { throw new TypeError('Failed to fetch'); } }; + await assert.rejects(rescopeImage(lost, OLD), TypeError, 'a lost answer is not final'); + for (const answer of [null, { success: false }, { success: true, image: { ...FRESH, root_scope: 'x' } }, + { success: true, image: { ...FRESH, path: 'other.png' } }, { success: true, image: { ...FRESH, root_index: -1 } }]) { + assert.equal(await rescopeImage(fakeApi(() => answer), OLD), null); + } + const api = fakeApi(() => ({ success: true, image: FRESH })); + for (const image of [null, { ...OLD, root_scope: 'short' }, { ...OLD, path: '' }, { ...OLD, root_index: 1.5 }]) { + assert.equal(await rescopeImage(api, image), null); + } + assert.equal(api.calls.length, 0, 'a malformed target is never sent'); +}); + +test('one rescope per held pair: callers share it, a tried pair answers its first result, reset forgets', async () => { + const { createSourceRecovery } = await load(); + let answer = { success: true, image: FRESH }; + const api = fakeApi(() => answer); + const applied = []; + const recovery = createSourceRecovery({ api, generation: () => 7, apply: async (image, fresh, started) => { applied.push([image.path, fresh.root_index, started]); return fresh; } }); + const [first, second] = await Promise.all([recovery.recover(OLD), recovery.recover({ ...OLD })]); + assert.deepEqual([first, second], [FRESH, FRESH]); + assert.equal(api.calls.length, 1); + assert.deepEqual(applied, [['sub/a.png', 1, 7]]); + answer = { success: false }; + assert.deepEqual(await recovery.recover(OLD), FRESH, 'the pair is not asked again'); + recovery.reset(); + assert.equal(await recovery.recover(OLD), null, 'after a reset the pair is asked again'); + assert.equal(api.calls.length, 2); + assert.equal(await recovery.recover({ ...OLD, root_scope: 'bad' }), null); + assert.equal(api.calls.length, 2); +}); + +test('a refused pair never loops, an aborted page applies nothing and a failing apply is null', async () => { + const { createSourceRecovery } = await load(); + const api = fakeApi(() => ({ success: true, image: FRESH })); + const page = new AbortController(); + let applied = 0; + const recovery = createSourceRecovery({ api, signal: page.signal, apply: async () => { applied += 1; return FRESH; } }); + page.abort(); + assert.equal(await recovery.recover(OLD), null); + assert.equal(applied, 0); + const failing = createSourceRecovery({ api, apply: async () => { throw new Error('page gone'); } }); + assert.equal(await failing.recover(OLD), null); + assert.throws(() => createSourceRecovery({ api }), TypeError); + // An api without post (an embedded gallery's page adapter) finds nothing. + assert.equal(await createSourceRecovery({ api: {}, apply: async () => FRESH }).recover(OLD), null); +}); + +test('the memo forgets its oldest pairs past its limit', async () => { + const { createSourceRecovery } = await load(); + const api = fakeApi(body => ({ success: true, image: { ...FRESH, path: body.target.path } })); + const recovery = createSourceRecovery({ api, apply: async (_image, fresh) => fresh }); + for (let index = 0; index < 65; index++) await recovery.recover({ ...OLD, path: `f${index}.png` }); + assert.equal(api.calls.length, 65); + await recovery.recover({ ...OLD, path: 'f64.png' }); + assert.equal(api.calls.length, 65, 'a recent pair is still remembered'); + await recovery.recover({ ...OLD, path: 'f0.png' }); + assert.equal(api.calls.length, 66, 'the oldest pair was forgotten'); +}); + +function fakePage({ selected = true, root = 'all', current = true } = {}) { + const log = []; + const page = { + filters: { root, subfolder: root === 'all' ? '' : 'sub' }, + current: started => { log.push(['current', started]); return current; }, + selected: image => selected && image.path === OLD.path, + inspect: image => log.push(['inspect', image.root_index]), + dropSelection: () => log.push(['drop']), + stream: { + refresh: async () => { log.push(['refresh']); throw new Error('read failed'); }, + reset: (filters, options) => log.push(['reset', { ...filters }, options]), + }, + updateScope: () => log.push(['scope']), + closeViewer: () => log.push(['close-viewer']), + clearChecks: () => log.push(['clear-checks']), + listsNow: () => true, + rediscover: () => log.push(['rediscover']), + }; + return { page, log }; +} + +test('a renumbered source keeps nothing by its old key: only the file stays selected', async () => { + const { recoveredSource } = await load(); + const { page, log } = fakePage({ root: '0' }); + assert.equal(await recoveredSource(page)(OLD, FRESH, 3), FRESH); + assert.deepEqual(log, [['current', 3], ['close-viewer'], ['clear-checks'], ['inspect', 1], ['scope'], + ['reset', { root: '1', subfolder: 'sub' }, { load: true }], ['rediscover']]); +}); + +test('another Source filter falls back to All sources; an unselected file drops the old selection', async () => { + const { recoveredSource } = await load(); + const { page, log } = fakePage({ root: '2', selected: false }); + await recoveredSource(page)(OLD, FRESH, 0); + assert.deepEqual(log.find(entry => entry[0] === 'reset')[1], { root: 'all', subfolder: '' }); + assert.ok(log.some(entry => entry[0] === 'drop')); + assert.ok(!log.some(entry => entry[0] === 'inspect')); + const all = fakePage({ root: 'all' }); + await recoveredSource(all.page)(OLD, FRESH, 0); + assert.deepEqual(all.log.find(entry => entry[0] === 'reset')[1], { root: 'all', subfolder: '' }); +}); + +test('an unchanged numbering re-reads in place; a page that moved on is left alone', async () => { + const { recoveredSource } = await load(); + const same = { ...FRESH, root_index: 0 }; + const { page, log } = fakePage(); + assert.equal(await recoveredSource(page)(OLD, same, 1), same); + assert.deepEqual(log, [['current', 1], ['inspect', 0], ['refresh']], 'a failed re-read is not an error'); + const unselected = fakePage({ selected: false }); + await recoveredSource(unselected.page)(OLD, same, 1); + assert.deepEqual(unselected.log, [['current', 1], ['refresh']]); + const moved = fakePage({ current: false }); + assert.equal(await recoveredSource(moved.page)(OLD, FRESH, 1), FRESH, 'the dialog may still retarget'); + assert.deepEqual(moved.log, [['current', 1]]); + await flush(); +}); + +test('a scope that still names its root is not stale: nothing is applied', async () => { + const { recoveredSource } = await load(); + const { page, log } = fakePage(); + assert.equal(await recoveredSource(page)(OLD, { ...OLD, url: '/u' }, 1), null); + assert.deepEqual(log, []); +}); + +test('a lost rescope answer is not remembered: the pair is asked again', async () => { + const { createSourceRecovery } = await load(); + let down = true; + const api = { calls: 0, post: async () => { api.calls += 1; if (down) throw Object.assign(new Error('bad gateway'), { status: 502 }); return { success: true, image: FRESH }; } }; + const recovery = createSourceRecovery({ api, apply: async (_image, fresh) => fresh }); + assert.equal(await recovery.recover(OLD), null); + down = false; + assert.deepEqual(await recovery.recover(OLD), FRESH); + assert.equal(api.calls, 2); +}); + +test('the same index under another scope (a changed key) re-reads in place and keeps the selection', async () => { + const { recoveredSource } = await load(); + const rekeyed = { ...OLD, root_scope: 'c'.repeat(64) }; + const { page, log } = fakePage(); + page.stream.refresh = async () => { log.push(['refresh']); }; + assert.equal(await recoveredSource(page)(OLD, rekeyed, 2), rekeyed); + assert.deepEqual(log, [['current', 2], ['inspect', 0], ['refresh']]); + assert.ok(!log.some(entry => ['reset', 'drop', 'clear-checks', 'close-viewer'].includes(entry[0]))); +}); diff --git a/tests/test_api_images.py b/tests/test_api_images.py index cc74c6cf..0775bcb1 100644 --- a/tests/test_api_images.py +++ b/tests/test_api_images.py @@ -2556,9 +2556,10 @@ class TestRootScopeServing(ImageAPITestCase): listing = await self.client.get("/prompt_manager/images/output?mode=workspace") descriptor = (await listing.json())["images"][0] self.assertRegex(descriptor.get("root_scope", ""), r"^[0-9a-f]{64}$") - with patch.object( - images_module, "_WORKSPACE_CURSOR_SECRET", b"different-process-key" - ): + from utils.workspace_keys import WorkspaceKeys + + rotated = WorkspaceKeys(b"n" * 32, b"m" * 32, False) + with patch.object(self.api, "_workspace_keys", return_value=rotated): await self.assert_no_access("image.png", scope=descriptor["root_scope"]) self.api._get_all_output_dirs = lambda: [] await self.assert_no_access("image.png", scope=descriptor["root_scope"]) diff --git a/tests/test_api_prompt_inspector.py b/tests/test_api_prompt_inspector.py index 8d301060..21ed8023 100644 --- a/tests/test_api_prompt_inspector.py +++ b/tests/test_api_prompt_inspector.py @@ -1418,9 +1418,10 @@ class TestPromptInspector(ImageAPITestCase): async def test_root_scope_rotated_key_and_removed_index_skip_reader_and_db(self): png(self.output_dir / "image.png", prompt=graph()) descriptor = await self.scoped_descriptor() - import py.api.images as images_module + from utils.workspace_keys import WorkspaceKeys - with patch.object(images_module, "_WORKSPACE_CURSOR_SECRET", b"new-key"): + rotated = WorkspaceKeys(b"n" * 32, b"m" * 32, False) + with patch.object(self.api, "_workspace_keys", return_value=rotated): await self.assert_scope_skips_access(descriptor.get("root_scope", "0" * 64)) self.api._get_all_output_dirs = lambda: [] await self.assert_scope_skips_access(descriptor.get("root_scope", "0" * 64)) diff --git a/tests/test_api_rescope.py b/tests/test_api_rescope.py new file mode 100644 index 00000000..f753b0ed --- /dev/null +++ b/tests/test_api_rescope.py @@ -0,0 +1,169 @@ +"""POST /prompt_manager/images/rescope: a stale target's current identity (#430). + +A page that holds ``{root_index, root_scope, path}`` after the sources were +renumbered asks where that file is now. The server maps the target only when +its scope was issued, at its old index, for one of the current roots, so the +answer names nothing the page could not already address; a removed or +replaced root, a foreign scope or a changed key stay refused. +""" + +import json +import os +import sys +from unittest.mock import patch + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +try: + from test_api_images import ImageAPITestCase, make_png +except ImportError: # imported as tests.test_api_rescope + from tests.test_api_images import ImageAPITestCase, make_png + +from utils.workspace_keys import WorkspaceKeys # noqa: E402 + +RESCOPE = "/prompt_manager/images/rescope" + + +class TestRescopeRoute(ImageAPITestCase): + async def get_application(self): + app = await super().get_application() + self.other_dir = self.comfy_dir / "other" + self.other_dir.mkdir() + make_png(self.output_dir / "sub" / "a.png") + make_png(self.other_dir / "sub" / "a.png", color=(0, 0, 255)) + return app + + def target(self, path="sub/a.png", index=0, root=None): + root = root or self.output_dir + return { + "root_index": index, + "root_scope": self.api._gallery_root_scope(root, index), + "path": path, + } + + async def post(self, body): + response = await self.client.post(RESCOPE, json=body) + data = await response.json() + self.assertNotIn(str(self.comfy_dir), json.dumps(data)) + self.assertEqual(response.headers.get("Cache-Control"), "no-store") + return response.status, data + + async def listed(self): + response = await self.client.get( + "/prompt_manager/images/output", params={"mode": "workspace"} + ) + return { + (item["root_index"], item["path"]): item + for item in (await response.json())["images"] + } + + async def test_an_unchanged_root_answers_the_listing_descriptor(self): + status, data = await self.post({"target": self.target()}) + self.assertEqual(status, 200) + self.assertTrue(data["success"]) + self.assertEqual(data["image"], (await self.listed())[(0, "sub/a.png")]) + + async def test_a_renumbered_root_maps_to_its_new_index(self): + held = self.target() + self.api._get_all_output_dirs = lambda: [self.other_dir, self.output_dir] + status, data = await self.post({"target": held}) + self.assertEqual(status, 200) + image = data["image"] + self.assertEqual((image["root_index"], image["path"]), (1, "sub/a.png")) + self.assertEqual( + image["root_scope"], self.api._gallery_root_scope(self.output_dir, 1) + ) + self.assertEqual(image, (await self.listed())[(1, "sub/a.png")]) + + async def test_a_removed_replaced_or_foreign_root_is_source_changed(self): + held = self.target() + cases = { + "removed": lambda: [], + "replaced": lambda: [self.other_dir], + } + for name, roots in cases.items(): + with self.subTest(name): + self.api._get_all_output_dirs = roots + status, data = await self.post({"target": held}) + self.assertEqual((status, data["code"]), (409, "source-changed")) + self.api._get_all_output_dirs = lambda: [self.output_dir] + for scope in ("0" * 64, self.api._gallery_root_scope(self.output_dir, 1)): + with self.subTest(scope=scope[:8]): + status, data = await self.post( + {"target": {**held, "root_scope": scope}} + ) + self.assertEqual((status, data["code"]), (409, "source-changed")) + + async def test_a_changed_key_cannot_be_mapped(self): + held = self.target() + rotated = WorkspaceKeys(b"n" * 32, b"m" * 32, False) + with patch.object(self.api, "_workspace_keys", return_value=rotated): + status, data = await self.post({"target": held}) + self.assertEqual((status, data["code"]), (409, "source-changed")) + + async def test_the_first_of_two_spellings_of_one_root_wins(self): + held = self.target() + self.api._get_all_output_dirs = lambda: [ + self.other_dir, + self.output_dir, + self.output_dir / "sub" / "..", + ] + status, data = await self.post({"target": held}) + self.assertEqual((status, data["image"]["root_index"]), (200, 1)) + + async def test_a_missing_file_is_not_found(self): + status, data = await self.post({"target": self.target("sub/gone.png")}) + self.assertEqual((status, data["code"]), (404, "file-missing")) + (self.output_dir / "dir.png").mkdir() + status, data = await self.post({"target": self.target("dir.png")}) + self.assertEqual((status, data["code"]), (404, "file-missing")) + + async def test_a_path_leaving_the_root_is_refused(self): + outside = make_png(self.comfy_dir / "outside.png") + self._make_symlink(outside, self.output_dir / "escape.png") + status, _data = await self.post({"target": self.target("escape.png")}) + self.assertEqual(status, 403) + + async def test_previews_are_not_rescoped(self): + make_png(self.output_dir / "thumbnails" / "a_thumb.png") + status, _data = await self.post( + {"target": self.target("thumbnails/a_thumb.png")} + ) + self.assertEqual(status, 403) + + async def test_malformed_bodies_are_refused_before_any_file_access(self): + good = self.target() + bodies = [ + b"{", + [], + {}, + {"target": good, "extra": 1}, + {"target": {**good, "root_index": -1}}, + {"target": {**good, "root_index": True}}, + {"target": {**good, "root_index": 4096}}, + # Past Python's int-to-text digit limit: never a 500. + b'{"target": {"root_index": 1' + b"0" * 5000 + b"}}", + {"target": {**good, "root_scope": "A" * 64}}, + {"target": {**good, "root_scope": "abc"}}, + {"target": {**good, "path": ""}}, + {"target": {**good, "path": "../a.png"}}, + {"target": {**good, "path": "/abs.png"}}, + {"target": {**good, "path": "C:/a.png"}}, + {"target": {**good, "path": "sub\\a.png"}}, + {"target": {**good, "path": "sub//a.png"}}, + {"target": {**good, "path": "notes.txt"}}, + {"target": {**good, "path": "x" * 2000 + ".png"}}, + ] + with patch("pathlib.Path.is_file", side_effect=AssertionError("file access")): + for body in bodies: + with self.subTest(body=str(body)[:60]): + if isinstance(body, bytes): + response = await self.client.post( + RESCOPE, + data=body, + headers={"Content-Type": "application/json"}, + ) + else: + response = await self.client.post(RESCOPE, json=body) + self.assertIn(response.status, (400, 403)) + self.assertFalse((await response.json())["success"]) diff --git a/tests/test_api_workspace_images.py b/tests/test_api_workspace_images.py index cd998722..82507e7a 100644 --- a/tests/test_api_workspace_images.py +++ b/tests/test_api_workspace_images.py @@ -1882,9 +1882,13 @@ class TestWorkspaceCursorEncoding(unittest.TestCase): for order in ("newest", "oldest", "name", "name-desc"): with self.subTest(sort=order): key = _workspace_key(entry, order) - token = _workspace_cursor(key, "scope") + token = _workspace_cursor(key, "scope", b"k" * 32) self.assertLessEqual(len(token), 4096) - self.assertEqual(_workspace_cursor_key(token, "scope", order), key) + self.assertEqual( + _workspace_cursor_key(token, "scope", order, b"k" * 32), key + ) + with self.assertRaises(ValueError): + _workspace_cursor_key(token, "scope", order, b"o" * 32) # The pre-#379 candidate read, kept as the differential reference for the diff --git a/tests/test_api_workspace_restart.py b/tests/test_api_workspace_restart.py new file mode 100644 index 00000000..d975c640 --- /dev/null +++ b/tests/test_api_workspace_restart.py @@ -0,0 +1,218 @@ +"""A ComfyUI restart keeps an open workspace page's scopes and cursors (#430). + +A restart is a new ``PromptManagerAPI`` on the same database in a new +process. Here a second API instance on its own aiohttp server stands in for +it (``tests/test_workspace_keys.py`` also derives the scope in a real child +process); it must accept every ``root_scope`` and listing cursor the first one +handed out, while a changed root list or key still refuses them. +""" + +import base64 +import json +import os +import sys +from unittest.mock import patch +from urllib.parse import quote + +from aiohttp import web +from aiohttp.test_utils import TestClient, TestServer + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +try: + from test_api_images import ImageAPITestCase, make_png +except ImportError: # imported as tests.test_api_workspace_restart + from tests.test_api_images import ImageAPITestCase, make_png + +from database.operations import PromptDatabase # noqa: E402 +from py.api import PromptManagerAPI # noqa: E402 +from utils.workspace_keys import KEY_FILE_NAME, WorkspaceKeys # noqa: E402 + +LISTING = "/prompt_manager/images/output" + + +def _payload(token): + raw = base64.urlsafe_b64decode(token + "=" * (-len(token) % 4)) + return json.loads(raw[32:]) + + +class TestRestartKeepsTheOpenPage(ImageAPITestCase): + async def get_application(self): + app = await super().get_application() + for index in range(3): + make_png(self.output_dir / f"image_{index}.png") + return app + + async def first_page(self, client=None, **params): + response = await (client or self.client).get( + LISTING, params={"mode": "workspace", "limit": "1", **params} + ) + self.assertEqual(response.status, 200) + return await response.json() + + async def restarted(self): + """A second API on the same database and roots, on its own server.""" + api = PromptManagerAPI(db=PromptDatabase(self._temp_db.name)) + api._get_all_output_dirs = lambda: [self.output_dir] + api._find_comfyui_output_dir = lambda: str(self.output_dir) + app = web.Application() + routes = web.RouteTableDef() + api.add_routes(routes) + app.router.add_routes(routes) + client = TestClient(TestServer(app)) + await client.start_server() + + async def close(): + await client.close() + api.close_auto_thumbnails() + api.db.close_all() + + self.addAsyncCleanup(close) + return api, client + + async def test_a_restart_keeps_every_held_scope_and_cursor_valid(self): + page = await self.first_page() + image, cursor = page["images"][0], page["next_cursor"] + self.assertTrue( + os.path.isfile( + os.path.join(os.path.dirname(self._temp_db.name), KEY_FILE_NAME) + ) + ) + _api, client = await self.restarted() + + more = await client.get( + LISTING, params={"mode": "workspace", "limit": "1", "cursor": cursor} + ) + self.assertEqual(more.status, 200, await more.text()) + self.assertNotEqual((await more.json())["images"][0]["path"], image["path"]) + + inspected = await client.get( + f"/prompt_manager/images/inspect/{quote(image['path'])}", + params={"root": "0", "root_scope": image["root_scope"]}, + ) + self.assertEqual(inspected.status, 200, await inspected.text()) + served = await client.get( + f"/prompt_manager/images/serve/{quote(image['path'])}", + params={"root": "0", "root_scope": image["root_scope"]}, + ) + self.assertEqual(served.status, 200) + relisted = await self.first_page(client) + self.assertEqual(relisted["images"][0]["root_scope"], image["root_scope"]) + + async def test_a_key_change_refuses_the_cursor_as_stale(self): + cursor = (await self.first_page())["next_cursor"] + rotated = WorkspaceKeys(b"n" * 32, b"m" * 32, False) + with patch.object(self.api, "_workspace_keys", return_value=rotated): + response = await self.client.get( + LISTING, params={"mode": "workspace", "limit": "1", "cursor": cursor} + ) + self.assertEqual(response.status, 400) + self.assertEqual((await response.json())["code"], "cursor-stale") + + async def test_renumbered_roots_refuse_the_cursor_as_stale(self): + cursor = (await self.first_page())["next_cursor"] + other = self.comfy_dir / "other" + other.mkdir() + self.api._get_all_output_dirs = lambda: [other, self.output_dir] + response = await self.client.get( + LISTING, params={"mode": "workspace", "limit": "1", "cursor": cursor} + ) + self.assertEqual(response.status, 400) + self.assertEqual((await response.json())["code"], "cursor-stale") + + async def test_another_listing_refuses_the_cursor_as_stale(self): + cursor = (await self.first_page())["next_cursor"] + response = await self.client.get( + LISTING, + params={"mode": "workspace", "limit": "1", "cursor": cursor, "q": "x"}, + ) + self.assertEqual(response.status, 400) + self.assertEqual((await response.json())["code"], "cursor-stale") + + async def test_a_malformed_cursor_is_a_plain_bad_request(self): + # A 32-byte signature over a payload is well formed: a wrong one is stale. + for cursor in ("!!", "a" * 5000, base64.urlsafe_b64encode(b"x" * 20).decode()): + with self.subTest(cursor=cursor[:10]): + response = await self.client.get( + LISTING, + params={"mode": "workspace", "limit": "1", "cursor": cursor}, + ) + self.assertEqual(response.status, 400) + self.assertNotEqual((await response.json()).get("code"), "cursor-stale") + + async def test_the_cursor_carries_no_digest_of_the_root_paths(self): + cursor = (await self.first_page())["next_cursor"] + payload = _payload(cursor) + # v2: the listing scope (it hashes every absolute root) is in the MAC only. + self.assertEqual(set(payload), {"v", "key"}) + self.assertEqual(payload["v"], 2) + + async def test_another_database_folder_gets_other_scopes(self): + page = await self.first_page() + other_db = self.comfy_dir / "elsewhere" / "prompts.db" + other_db.parent.mkdir() + api = PromptManagerAPI(db=PromptDatabase(str(other_db))) + self.addCleanup(api.db.close_all) + self.addCleanup(api.close_auto_thumbnails) + self.assertNotEqual( + api._gallery_root_scope(self.output_dir, 0), page["images"][0]["root_scope"] + ) + + +# A fresh interpreter: the API on the given database, as after a restart. +_CHILD = """ +import json, sys +from pathlib import Path +from unittest.mock import MagicMock +sys.path.insert(0, sys.argv[1]) +from tests.host_stubs import install_folder_paths_stub +install_folder_paths_stub() +sys.modules.setdefault("server", MagicMock()) +from database.operations import PromptDatabase +from py.api import PromptManagerAPI +from py.api.images import _workspace_cursor_key +api = PromptManagerAPI(db=PromptDatabase(sys.argv[2])) +scope = api._gallery_root_scope(Path(sys.argv[3]), 0) +secret = api._workspace_keys().cursor +key = _workspace_cursor_key(sys.argv[4], "s" * 64, "newest", secret) +api.close_auto_thumbnails() +api.db.close_all() +print(json.dumps({"scope": scope, "key": list(key)})) +""" + + +class TestRestartInANewProcess(ImageAPITestCase): + """The real thing: two other interpreters on the same database.""" + + def child(self, cursor): + import subprocess + + repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + result = subprocess.run( + [ + sys.executable, + "-c", + _CHILD, + repo, + self._temp_db.name, + str(self.output_dir), + cursor, + ], + cwd=self.comfy_dir, + capture_output=True, + text=True, + timeout=120, + ) + self.assertEqual(result.returncode, 0, result.stderr[-2000:]) + return json.loads(result.stdout.strip().splitlines()[-1]) + + async def test_new_processes_derive_the_same_scope_and_accept_the_cursor(self): + from py.api.images import _workspace_cursor + + make_png(self.output_dir / "image.png") + listing = await self.client.get(LISTING, params={"mode": "workspace"}) + scope = (await listing.json())["images"][0]["root_scope"] + key = (-12.5, "image.png", "0" * 64, 0) + cursor = _workspace_cursor(key, "s" * 64, self.api._workspace_keys().cursor) + for _restart in range(2): + self.assertEqual(self.child(cursor), {"scope": scope, "key": list(key)}) diff --git a/tests/test_library_move.py b/tests/test_library_move.py index b838af3a..9a450fde 100644 --- a/tests/test_library_move.py +++ b/tests/test_library_move.py @@ -209,6 +209,38 @@ class TestMove(_MoveCase): ) self.assertEqual(ll.read_marker(self.target)["state"], ll.MARKER_MOVED) + def test_the_workspace_key_moves_with_the_library(self): + # #430: open pages keep their scopes after the restart that follows. + from utils.workspace_keys import KEY_FILE_NAME, load_install_secret + + secret = load_install_secret(self.ext) + result = self.move() + self.assertIn(KEY_FILE_NAME, result.files) + self.assertEqual(load_install_secret(self.target), secret) + if os.name == "posix": + mode = os.stat(os.path.join(self.target, KEY_FILE_NAME)).st_mode + self.assertEqual(mode & 0o777, 0o600) + + def test_a_key_that_cannot_be_copied_never_fails_the_move(self): + from utils.workspace_keys import KEY_FILE_NAME, load_install_secret + + load_install_secret(self.ext) + real = lm._hash_copy + + def failing(source, destination): + if os.path.basename(source) == KEY_FILE_NAME: + raise OSError("disk error") + return real(source, destination) + + with mock.patch.object(lm, "_hash_copy", failing): + with self.assertLogs("prompt_manager.database.library_move", "WARNING"): + result = self.move() + self.assertNotIn(KEY_FILE_NAME, result.files) + self.assertEqual(_rows(self.new_db), self.before["rows"]) + self.assertFalse( + any(n.startswith(lm.TEMP_PREFIX) for n in os.listdir(self.target)) + ) + def test_new_files_are_private(self): if os.name != "posix": self.skipTest("POSIX permissions") diff --git a/tests/test_workspace_keys.py b/tests/test_workspace_keys.py new file mode 100644 index 00000000..b3ed70da --- /dev/null +++ b/tests/test_workspace_keys.py @@ -0,0 +1,247 @@ +"""The per-install workspace secret (#430, utils/workspace_keys.py). + +A ComfyUI restart must not change the gallery root scopes or the workspace +cursors while the output roots stay the same, so the secret that keys them is +persisted beside the database instead of drawn per process. +""" + +import importlib +import json +import logging +import os +import stat +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import utils.workspace_keys as workspace_keys # noqa: E402 +from utils.workspace_keys import ( # noqa: E402 + KEY_FILE_NAME, + WorkspaceKeys, + gallery_root_scope, + load_install_secret, + workspace_keys_for, +) + +REPO = Path(__file__).resolve().parents[1] + + +class TempFolderCase(unittest.TestCase): + def setUp(self): + folder = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + self.addCleanup(folder.cleanup) + self.folder = Path(os.path.realpath(folder.name)) + self.key_file = self.folder / KEY_FILE_NAME + + def leftovers(self): + return sorted(p.name for p in self.folder.iterdir() if p.name != KEY_FILE_NAME) + + +class TestLoadInstallSecret(TempFolderCase): + def test_a_missing_file_is_created_with_64_hex_and_owner_only_mode(self): + secret = load_install_secret(str(self.folder)) + self.assertEqual(len(secret), 32) + text = self.key_file.read_text(encoding="ascii") + self.assertEqual(text, secret.hex() + "\n") + self.assertEqual(self.leftovers(), [], "no temp file is left behind") + if os.name == "posix": + self.assertEqual(stat.S_IMODE(self.key_file.stat().st_mode) & 0o077, 0) + + def test_an_existing_file_is_reused(self): + first = load_install_secret(str(self.folder)) + self.assertEqual(load_install_secret(str(self.folder)), first) + + def test_an_uppercase_or_unterminated_file_is_still_read(self): + self.key_file.write_text("AB" * 32, encoding="ascii") + self.assertEqual( + load_install_secret(str(self.folder)), bytes.fromhex("ab" * 32) + ) + + def test_a_corrupt_file_is_replaced_with_a_warning(self): + for content in (b"", b"short", b"zz" * 32, b"ab" * 33, b"\xff" * 64): + with self.subTest(content=content): + self.key_file.write_bytes(content) + with self.assertLogs("prompt_manager.api", logging.WARNING) as logs: + secret = load_install_secret(str(self.folder)) + self.assertEqual(len(secret), 32) + self.assertEqual( + self.key_file.read_text(encoding="ascii"), secret.hex() + "\n" + ) + self.assertIn("replaced", "\n".join(logs.output)) + self.assertNotIn(secret.hex(), "\n".join(logs.output)) + self.assertEqual(self.leftovers(), []) + + def test_a_lost_creation_race_reads_the_winners_file(self): + winner = "cd" * 32 + "\n" + real_link = os.link + + def racing_link(source, target, *args, **kwargs): + Path(target).write_text(winner, encoding="ascii") + return real_link(source, target, *args, **kwargs) # FileExistsError + + with patch.object(workspace_keys.os, "link", racing_link): + secret = load_install_secret(str(self.folder)) + self.assertEqual(secret, bytes.fromhex("cd" * 32)) + self.assertEqual(self.key_file.read_text(encoding="ascii"), winner) + self.assertEqual(self.leftovers(), []) + + def test_a_file_system_without_hard_links_falls_back_to_replace(self): + def no_link(*args, **kwargs): + raise OSError(1, "Operation not permitted") + + with patch.object(workspace_keys.os, "link", no_link): + secret = load_install_secret(str(self.folder)) + self.assertEqual(self.key_file.read_text(encoding="ascii"), secret.hex() + "\n") + self.assertEqual(self.leftovers(), []) + + def test_an_unwritable_folder_returns_none_and_leaves_nothing(self): + def refuse(*args, **kwargs): + raise PermissionError(13, "Permission denied") + + with patch.object(workspace_keys.os, "open", refuse): + with self.assertLogs("prompt_manager.api", logging.WARNING): + self.assertIsNone(load_install_secret(str(self.folder))) + self.assertFalse(self.key_file.exists()) + with self.assertLogs("prompt_manager.api", logging.WARNING): + self.assertIsNone(load_install_secret(str(self.folder / "missing"))) + + def test_an_unreadable_key_path_returns_none(self): + self.key_file.mkdir() # a folder where the file should be + with self.assertLogs("prompt_manager.api", logging.WARNING): + self.assertIsNone(load_install_secret(str(self.folder))) + + def test_a_refused_replace_of_a_corrupt_key_falls_back(self): + # Windows refuses os.replace while another process holds the file open. + self.key_file.write_text("broken", encoding="ascii") + + def refuse(*args, **kwargs): + raise PermissionError(13, "Access is denied") + + with patch.object(workspace_keys.os, "replace", refuse): + with self.assertLogs("prompt_manager.api", logging.WARNING): + self.assertIsNone(load_install_secret(str(self.folder))) + self.assertEqual(self.key_file.read_text(encoding="ascii"), "broken") + self.assertEqual(self.leftovers(), []) + + def test_a_lost_race_against_an_unfinished_file_is_unusable_not_replaced(self): + def racing_link(source, target, *args, **kwargs): + Path(target).write_text("ab", encoding="ascii") # still being written + raise FileExistsError(target) + + with patch.object(workspace_keys.os, "link", racing_link): + with self.assertLogs("prompt_manager.api", logging.WARNING): + self.assertIsNone(load_install_secret(str(self.folder))) + self.assertEqual(self.key_file.read_text(encoding="ascii"), "ab") + + @unittest.skipUnless(os.name == "posix", "POSIX permission bits") + def test_an_existing_key_readable_by_others_is_tightened(self): + secret = load_install_secret(str(self.folder)) + os.chmod(self.key_file, 0o644) + self.assertEqual(load_install_secret(str(self.folder)), secret) + self.assertEqual(stat.S_IMODE(self.key_file.stat().st_mode), 0o600) + + def test_old_temp_files_are_swept_and_fresh_ones_kept(self): + old = self.folder / f".{KEY_FILE_NAME}.dead.tmp" + fresh = self.folder / f".{KEY_FILE_NAME}.live.tmp" + other = self.folder / "prompts.db" + for path in (old, fresh, other): + path.write_text("x") + os.utime(old, (1, 1)) + load_install_secret(str(self.folder)) + self.assertEqual(self.leftovers(), sorted([fresh.name, other.name])) + + +class TestWorkspaceKeys(TempFolderCase): + def test_purposes_are_domain_separated_and_stable(self): + keys = workspace_keys_for(str(self.folder / "prompts.db")) + self.assertTrue(keys.persisted) + self.assertNotEqual(keys.root_scope, keys.cursor) + again = workspace_keys_for(str(self.folder / "prompts.db")) + self.assertEqual( + (again.root_scope, again.cursor), (keys.root_scope, keys.cursor) + ) + self.assertNotIn( + self.key_file.read_text().strip(), + (keys.root_scope.hex(), keys.cursor.hex()), + ) + + def test_another_database_folder_gets_other_keys(self): + other = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + self.addCleanup(other.cleanup) + first = workspace_keys_for(str(self.folder / "prompts.db")) + second = workspace_keys_for(os.path.join(other.name, "prompts.db")) + self.assertNotEqual(first.root_scope, second.root_scope) + + def test_an_unusable_folder_falls_back_to_a_process_key_with_a_warning(self): + with patch.object(workspace_keys, "load_install_secret", return_value=None): + with self.assertLogs("prompt_manager.api", logging.WARNING) as logs: + keys = workspace_keys_for(str(self.folder / "prompts.db")) + self.assertFalse(keys.persisted) + self.assertEqual(len(keys.root_scope), 32) + self.assertIn("restart", "\n".join(logs.output)) + + def test_memory_and_blank_paths_never_write_into_the_working_folder(self): + for path in (":memory:", "", None): + with self.subTest(path=path): + with patch.object(workspace_keys, "load_install_secret") as load: + with self.assertLogs("prompt_manager.api", logging.WARNING): + keys = workspace_keys_for(path) + load.assert_not_called() + self.assertFalse(keys.persisted) + + def test_the_scope_is_the_historic_payload_under_the_root_scope_key(self): + import hashlib + import hmac + + keys = WorkspaceKeys(b"r" * 32, b"c" * 32, True) + payload = json.dumps( + ["prompt-manager-gallery-root-v1", 3, "/x/ü"], + separators=(",", ":"), + ensure_ascii=False, + ).encode("utf-8") + self.assertEqual( + gallery_root_scope(keys, "/x/ü", 3), + hmac.new(b"r" * 32, payload, hashlib.sha256).hexdigest(), + ) + + +class TestRestartKeepsScopes(TempFolderCase): + """A new process or a reloaded module derives the same scope (#430).""" + + def test_a_module_reload_keeps_the_scope(self): + db = str(self.folder / "prompts.db") + before = gallery_root_scope(workspace_keys_for(db), "/out", 0) + reloaded = importlib.reload(workspace_keys) + self.addCleanup(importlib.reload, workspace_keys) + self.assertEqual( + reloaded.gallery_root_scope(reloaded.workspace_keys_for(db), "/out", 0), + before, + ) + + def test_a_new_process_derives_the_same_scope(self): + db = str(self.folder / "prompts.db") + before = gallery_root_scope(workspace_keys_for(db), "/out", 0) + code = ( + "import sys\n" + "from utils.workspace_keys import gallery_root_scope, workspace_keys_for\n" + "print(gallery_root_scope(workspace_keys_for(sys.argv[1]), '/out', 0))\n" + ) + # -I would drop the repo from sys.path; the child imports only stdlib code. + result = subprocess.run( + [sys.executable, "-c", code, db], + cwd=REPO, + capture_output=True, + text=True, + timeout=60, + check=True, + ) + self.assertEqual(result.stdout.strip(), before) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/vendor/frontend-coverage-baseline.json b/tools/vendor/frontend-coverage-baseline.json index dd070d4b..72311876 100644 --- a/tools/vendor/frontend-coverage-baseline.json +++ b/tools/vendor/frontend-coverage-baseline.json @@ -1,9 +1,9 @@ { "admin.js": { - "lines": 37, - "branches": 61, - "functions": 32, - "statements": 37 + "lines": 47, + "branches": 64, + "functions": 50, + "statements": 47 }, "api-client.js": { "lines": 98, @@ -38,7 +38,7 @@ "focused-node.js": { "lines": 97, "branches": 87, - "functions": 90, + "functions": 91, "statements": 97 }, "gallery-settings.js": { @@ -48,10 +48,10 @@ "statements": 98 }, "gallery.js": { - "lines": 49, - "branches": 68, - "functions": 50, - "statements": 49 + "lines": 54, + "branches": 72, + "functions": 57, + "statements": 54 }, "html-escape.js": { "lines": 98, @@ -61,7 +61,7 @@ }, "image-helpers.js": { "lines": 96, - "branches": 91, + "branches": 92, "functions": 98, "statements": 96 }, @@ -260,7 +260,7 @@ "workspace/image-tagging.mjs": { "lines": 97, "branches": 92, - "functions": 88, + "functions": 89, "statements": 97 }, "workspace/images-category.mjs": { @@ -293,6 +293,12 @@ "functions": 98, "statements": 98 }, + "workspace/images-source-recovery.mjs": { + "lines": 98, + "branches": 98, + "functions": 98, + "statements": 98 + }, "workspace/images-state.mjs": { "lines": 98, "branches": 98, @@ -409,7 +415,7 @@ }, "workspace/local-resolver.mjs": { "lines": 96, - "branches": 86, + "branches": 87, "functions": 98, "statements": 96 }, @@ -529,7 +535,7 @@ }, "workspace/system-diagnostics.mjs": { "lines": 95, - "branches": 86, + "branches": 87, "functions": 94, "statements": 95 }, @@ -625,7 +631,7 @@ }, "workspace/vocabulary-dialog.mjs": { "lines": 98, - "branches": 88, + "branches": 89, "functions": 95, "statements": 98 }, @@ -638,7 +644,7 @@ "workspace/vocabulary.mjs": { "lines": 98, "branches": 91, - "functions": 93, + "functions": 98, "statements": 98 }, "workspace/workspace-dialogs.mjs": { diff --git a/utils/workspace_keys.py b/utils/workspace_keys.py new file mode 100644 index 00000000..5087c53b --- /dev/null +++ b/utils/workspace_keys.py @@ -0,0 +1,213 @@ +"""The per-install secret behind the workspace's gallery scopes and cursors (#430). + +A page addresses a listed file as ``{root_index, root_scope, path}``. The root +scope is an HMAC over the root's index and canonical path, so the server can +refuse an action aimed at a root that was renumbered or replaced since the page +listed it (``py/api/images.py`` ``_gallery_scope_error``). The listing's keyset +cursors are MACed with a second key, so a forged position never reaches the +sort code. + +Both keys used to be drawn per process, so every ComfyUI restart made every +open workspace tab stale. They now derive from one random secret persisted in +``workspace-scope.key`` beside the database (never in ``config.json`` and never +in the database, so a backup or an export does not carry it). When the folder +cannot hold it, the process falls back to a per-process secret, as before. + +Standard library only: a fresh process can derive the same scope without +importing the API. +""" + +from __future__ import annotations + +import hashlib +import hmac +import json +import logging +import os +import re +import secrets +import stat +import time +from typing import NamedTuple + +logger = logging.getLogger("prompt_manager.api") + +KEY_FILE_NAME = "workspace-scope.key" +SECRET_BYTES = 32 +_HEX = re.compile(r"[0-9a-fA-F]{64}") +# Bounded read: a valid file is 64 hex characters and a newline. +_MAX_FILE_BYTES = 128 +_ROOT_SCOPE_PURPOSE = b"prompt-manager/gallery-root-scope/v1" +_CURSOR_PURPOSE = b"prompt-manager/workspace-cursor/v1" +_SCOPE_TAG = "prompt-manager-gallery-root-v1" +_TEMP_PREFIX = f".{KEY_FILE_NAME}." +_TEMP_SUFFIX = ".tmp" +_STALE_TEMP_SECONDS = 60 + + +class WorkspaceKeys(NamedTuple): + """The derived keys; ``persisted`` is False for a per-process fallback.""" + + root_scope: bytes + cursor: bytes + persisted: bool + + +def _read(path: str) -> bytes | None: + """The secret in *path*, None when it is missing or not 64 hex characters. + + Raises OSError for anything but a missing file (a folder, no permission). + """ + try: + with open(path, "rb") as handle: + raw = handle.read(_MAX_FILE_BYTES + 1) + except FileNotFoundError: + return None + try: + text = raw.decode("ascii").strip() + except UnicodeDecodeError: + return b"" + return bytes.fromhex(text) if _HEX.fullmatch(text) else b"" + + +def _write_exclusive(path: str, secret: bytes) -> None: + """Create *path* (it must not exist) holding *secret*, owner-only, fsynced.""" + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0) + descriptor = os.open(path, flags, 0o600) + try: + with os.fdopen(descriptor, "wb") as handle: + handle.write((secret.hex() + "\n").encode("ascii")) + handle.flush() + os.fsync(handle.fileno()) + except BaseException: + _unlink(path) + raise + + +def _unlink(path: str) -> None: + try: + os.unlink(path) + except OSError: + pass + + +def _sweep_temps(directory: str) -> None: + """Remove temp files a crashed first start left (older than a minute).""" + try: + names = os.listdir(directory) + except OSError: + return + cutoff = time.time() - _STALE_TEMP_SECONDS + for name in names: + if name.startswith(_TEMP_PREFIX) and name.endswith(_TEMP_SUFFIX): + path = os.path.join(directory, name) + try: + if os.stat(path).st_mtime < cutoff: + os.unlink(path) + except OSError: + pass + + +def _tighten(path: str) -> None: + """On POSIX an existing key readable by group or others becomes 0o600.""" + if os.name != "posix": + return # the file inherits the folder's ACL, as the database does + try: + if stat.S_IMODE(os.stat(path).st_mode) & 0o077: + os.chmod(path, 0o600) + except OSError: + pass + + +def _store(directory: str, path: str, secret: bytes, *, replace: bool) -> None: + """Publish *secret* at *path*; a new file never overwrites a concurrent winner. + + The secret is written to a temp file first. ``os.link`` then fails when + *path* exists, so of two first starts exactly one file wins + (FileExistsError for the other). A file system without hard links (FAT, + some shares) creates *path* with ``O_EXCL`` instead. *replace* (a corrupt + key) swaps the file with ``os.replace``; Windows refuses that while + another process holds the file open, which the caller treats as unusable. + """ + temp = os.path.join( + directory, f"{_TEMP_PREFIX}{secrets.token_hex(8)}{_TEMP_SUFFIX}" + ) + _write_exclusive(temp, secret) + try: + if replace: + os.replace(temp, path) + return + try: + os.link(temp, path) + except FileExistsError: + raise + except OSError: + # Not atomic: a second first start reading mid-write would see a + # corrupt key. Rare (two first starts on FAT or a share at once); + # each process still uses the file it reads back. + _write_exclusive(path, secret) + finally: + _unlink(temp) + + +def load_install_secret(directory: str) -> bytes | None: + """The persisted secret of *directory*, created on first use; None if unusable. + + After any write the file is read back and its content is what counts, so + processes racing on a first start end with the one secret on disk. + """ + path = os.path.join(directory, KEY_FILE_NAME) + try: + _sweep_temps(directory) + current = _read(path) + if current: + _tighten(path) + return current + corrupt = current is not None + try: + _store(directory, path, secrets.token_bytes(SECRET_BYTES), replace=corrupt) + except FileExistsError: + pass # another process created it first: its file counts + stored = _read(path) + if not stored: + raise OSError(f"{KEY_FILE_NAME} is not readable after it was written") + if corrupt: + logger.warning( + f"{KEY_FILE_NAME} was not a valid key and was replaced; " + "open workspace pages must refresh once" + ) + return stored + except OSError as exc: + logger.warning(f"Could not use {KEY_FILE_NAME}: {exc.__class__.__name__}") + return None + + +def _derive(secret: bytes, purpose: bytes) -> bytes: + return hmac.new(secret, purpose, hashlib.sha256).digest() + + +def workspace_keys_for(db_path: str | None) -> WorkspaceKeys: + """The keys for the library whose database is *db_path* (blocking I/O).""" + secret = None + if isinstance(db_path, str) and db_path.strip() and db_path != ":memory:": + secret = load_install_secret(os.path.dirname(os.path.abspath(db_path))) + persisted = secret is not None + if not persisted: + logger.warning( + "Workspace scopes use a per-process key: open workspace pages must " + "refresh after a restart" + ) + secret = secrets.token_bytes(SECRET_BYTES) + return WorkspaceKeys( + _derive(secret, _ROOT_SCOPE_PURPOSE), + _derive(secret, _CURSOR_PURPOSE), + persisted, + ) + + +def gallery_root_scope(keys: WorkspaceKeys, canonical_root: str, index: int) -> str: + """The 64-hex scope of the root at *index* (its canonical path string).""" + payload = json.dumps( + [_SCOPE_TAG, index, canonical_root], separators=(",", ":"), ensure_ascii=False + ).encode("utf-8") + return hmac.new(keys.root_scope, payload, hashlib.sha256).hexdigest() diff --git a/web/js/workspace/image-tagging-state.mjs b/web/js/workspace/image-tagging-state.mjs index 18b2e880..c4c44139 100644 --- a/web/js/workspace/image-tagging-state.mjs +++ b/web/js/workspace/image-tagging-state.mjs @@ -34,9 +34,13 @@ const MAX_SUGGESTIONS = 100; const MAX_INSTRUCTION_CHARS = 4000; const EDITABLE = new Set(['ready', 'generate-error', 'write-error', 'conflict']); // Answers after which only a fresh observation can save (the capture or epoch is gone); Retry observes again (D116). -const REOPEN = {'storage-changed': 'storage-changed', 'file-changed': 'file-changed', 'source-changed': 'file-changed', 'capture-expired': 'capture-expired'}; -const REOBSERVE = new Set(Object.values(REOPEN)); +const REOBSERVE = new Set(['storage-changed', 'file-changed', 'capture-expired']); +// #430: a changed source (the page's root scope no longer names its root) is never retried as is: the page re-scopes +// the target (`retarget`) or the dialog offers a refresh. +const REOPEN = Object.freeze({...Object.fromEntries([...REOBSERVE].map(code => [code, code])), 'source-changed': 'source-changed'}); const RETRYABLE = new Set(['observe-error', 'generate-error', 'unknown-outcome', ...REOBSERVE]); +// States whose reason blocks Save (and Generate): the dialog shows it beside its actions (D147). +const BLOCKED = new Set(['observe-error', 'source-changed', 'unknown-outcome', ...REOBSERVE]); // Generic failure envelopes whose fixed sentence the dialog words itself. const GENERIC_CODES = new Set(['observe-failed', 'suggest-failed', 'save-failed', 'capture-error']); // A proxy timeout or bad gateway cannot tell whether the server committed the write. @@ -238,6 +242,7 @@ function derive(state) { canSave: editable, canGenerate: editable && state.generateSupported, canRetry: RETRYABLE.has(state.phase), + blocked: BLOCKED.has(state.phase), }; } @@ -260,7 +265,7 @@ function validatePreset(preset) { */ export function createImageTaggingController({api, target, source = 'images', onLateSave = () => {}, preset = null} = {}) { requireValue(object(api) && ['get', 'post', 'put', 'del'].every(name => typeof api[name] === 'function'), 'An api client is required'); - const imageTarget = validateTarget(target); + let imageTarget = validateTarget(target); requireValue(SOURCES.has(source), 'Unknown tagging source'); requireValue(typeof onLateSave === 'function', 'onLateSave must be a function'); const chosen = validatePreset(preset); @@ -333,6 +338,10 @@ export function createImageTaggingController({api, target, source = 'images', on } } catch (error) { if (closed || serial !== openSerial) return; + if (isApiError(error) && error.code === 'source-changed') { + set({phase: 'source-changed', statusText: error.message}); + return; + } set({phase: 'observe-error', statusText: messageOf(error, COPY.observeFailed)}); return; } @@ -500,6 +509,16 @@ export function createImageTaggingController({api, target, source = 'images', on return send(lastBody); } + /** + * #430: the page found the file under a fresh `target` (its sources were renumbered): observe it there, keeping the + * typed draft. Refused (false) while a save is in flight or after close; a Generate for the old target is dropped. + */ + function retarget(next) { + if (closed || saving) return Promise.resolve(false); + imageTarget = validateTarget(next); + return open({keepDraft: observedOnce || Boolean(state.manual)}).then(() => true); + } + function cancel() { if (closed) return; set({closed: true}); @@ -525,7 +544,8 @@ export function createImageTaggingController({api, target, source = 'images', on return { get state() {return state;}, - subscribe, open, setManual, setModel, setOption, generate, remove, save, retry, cancel, dispose, + get target() {return imageTarget;}, + subscribe, open, setManual, setModel, setOption, generate, remove, save, retry, retarget, cancel, dispose, }; } diff --git a/web/js/workspace/image-tagging.mjs b/web/js/workspace/image-tagging.mjs index 52d31c20..d02699be 100644 --- a/web/js/workspace/image-tagging.mjs +++ b/web/js/workspace/image-tagging.mjs @@ -24,6 +24,9 @@ import { showWorkspaceToast } from './toast.mjs'; import { cardTitle } from './images-state.mjs'; const MANUAL_HELP = 'Comma-separated. Generated suggestions are added only after you approve them.'; +// #430: while the page looks for a renumbered source's file, and the preview that a refused capture leaves. +const FINDING_SOURCE = 'The source of this image changed. Finding it in the refreshed image collection…'; +const PREVIEW_UNAVAILABLE = 'Preview unavailable'; const SCOPE_NOTE = 'Tags belong to this image. Resolve its prompt association separately in Needs review.'; // The option inputs per controller key; the classifier keys only exist for WD14 models. const OPTION_IDS = Object.freeze({ @@ -194,6 +197,13 @@ function previewOf(image, state) { if (video) { node.muted = true; node.preload = 'metadata'; node.playsInline = true; } return node; } +/** D147: the preview box a failed or refused capture leaves, saying so (the reason is beside the actions). */ +function unavailablePreview() { + const node = el('div', PREVIEW_UNAVAILABLE, 'tag-image-preview tag-preview-empty'); + node.setAttribute('role', 'img'); + node.setAttribute('aria-label', PREVIEW_UNAVAILABLE); + return node; +} /** * Open "Tag image" for one listed `image` (its tagging target). `source` is the page (`images`, `library`, `review`, @@ -207,7 +217,8 @@ function previewOf(image, state) { * Escape and × write nothing and call `onStop` once. Swapping the next image's dialog in is not a stop. */ export function openImageTagging({api, scope, image, source = 'images', trigger = null, onSaved = null, preset = null, - autoGenerate = false, actions: mode = 'tag', position = null, onSkip = null, onStop = null}) { + autoGenerate = false, actions: mode = 'tag', position = null, onSkip = null, onStop = null, onSourceChanged = null, + onRefresh = null}) { const target = imageTarget(image); if (!target) throw new TypeError('The image has no tagging target'); const review = mode === 'review'; @@ -216,7 +227,8 @@ export function openImageTagging({api, scope, image, source = 'images', trigger // Set once this dialog hands over to the session (Approve, Skip): its close is then not a Stop. let advancing = false; const saved = ({filename: name, tags, toast}) => { - const event = {target, filename: name, tags, source}; + // A retargeted dialog (#430) reports the file under its current identity. + const event = {target: {...controller.target}, filename: name, tags, source}; announceSaved(event); if (toast) showWorkspaceToast(toast); onSaved?.(event); @@ -280,6 +292,15 @@ export function openImageTagging({api, scope, image, source = 'images', trigger : (last ? 'Save and finish' : 'Save and next')); if (review) actions.append(cancel, skip, primary); else actions.append(cancel, primary); + // D147: the reason a state blocks Save (and Generate), on its own row above the buttons; for a changed source the + // page is asked to find the file again (`onSourceChanged`), else Refresh is offered. + const notice = el('div', undefined, 'tag-action-notice'); + const noticeText = el('span'); + noticeText.setAttribute('role', 'status'); + const refresh = createButton(onRefresh ? 'Refresh image collection' : 'Reload page'); + notice.append(noticeText); + const tried = new Set(); + let finding = false, current = image; const nodes = [heading]; if (controller.state.showScopeNote) nodes.push(el('p', SCOPE_NOTE, 'tag-scope-note small')); @@ -334,8 +355,45 @@ export function openImageTagging({api, scope, image, source = 'images', trigger closeWorkspaceDialog(); saved({filename: state.image.filename, tags: state.savedTags.slice(), toast: state.toast}); } + /** #430: ask the page once per target where a renumbered source's file is now; a found file is retargeted. */ + function findSource() { + const {root_index: index, root_scope: held} = controller.target; + const attempt = `${index}:${held}`; + if (!onSourceChanged || tried.has(attempt)) return; + tried.add(attempt); + finding = true; + Promise.resolve().then(() => onSourceChanged({...current, ...controller.target})).catch(() => null).then(fresh => { + finding = false; + if (!open || completed) return; + const next = imageTarget(fresh); + if (next && controller.state.phase === 'source-changed') { + current = fresh; + controller.retarget(next); + } else render(controller.state); + }); + } + function renderNotice(state) { + const changed = state.phase === 'source-changed'; + const text = !state.blocked ? '' : changed && finding ? FINDING_SOURCE : state.statusText; + setText(noticeText, text); + const offer = changed && !finding; + if (offer !== refresh.isConnected) { + if (offer) notice.append(refresh); + else refresh.remove(); + } + // Its own first row; Retry still leads the buttons after it (D116). + if (text && actions.firstElementChild !== notice) actions.prepend(notice); + else if (!text && notice.isConnected) notice.remove(); + // The empty preview of a refused or failed capture says why it is empty. + if (state.blocked && !state.image && preview.tagName !== 'DIV') { + const next = unavailablePreview(); + preview.replaceWith(next); + preview = next; + } + } function render(state) { if (!open || completed) return; + if (state.phase === 'source-changed' && !finding) findSource(); if (state.image && state.image !== shownImage) renderImage(state.image); // The observation's saved tags, a kept draft, or a conflict's current set ahead of the draft (I2). if (state.draftRevision !== shownDraft) { @@ -344,7 +402,8 @@ export function openImageTagging({api, scope, image, source = 'images', trigger } if (state.suggestions !== shownRows || state.suggestionsText !== shownText) renderSuggestions(state); setText(count, state.countText); - setText(status, state.statusText); + // A blocking reason is shown once, beside the actions (D147). + setText(status, state.blocked ? '' : state.statusText); setText(primary, review ? reviewLabel(state) : state.primaryLabel); setDisabled(primary, !state.canSave); // D90: while Approve's save is in flight the session cannot be left, so its count includes that save. @@ -356,10 +415,16 @@ export function openImageTagging({api, scope, image, source = 'images', trigger retry.remove(); } } + renderNotice(state); if (state.phase === 'saved') complete(state); } cancel.addEventListener('click', () => { if (!disabled(cancel)) closeWorkspaceDialog(); }); + refresh.addEventListener('click', () => { + closeWorkspaceDialog(); + if (onRefresh) onRefresh(); + else window.location.reload(); + }); skip.addEventListener('click', () => { if (!open || advancing || disabled(skip)) return; advancing = true; diff --git a/web/js/workspace/images-source-recovery.mjs b/web/js/workspace/images-source-recovery.mjs new file mode 100644 index 00000000..b8f686ce --- /dev/null +++ b/web/js/workspace/images-source-recovery.mjs @@ -0,0 +1,96 @@ +/** + * Finding a file again after its source changed (#430). + * + * A listed file is addressed as `{root_index, root_scope, path}`. When the sources are renumbered every per-file + * action on it answers 409 `source-changed`. `rescopeImage` asks the server (POST /prompt_manager/images/rescope) + * for the file's current listing descriptor: the server maps the target only when its scope was issued for one of + * the current roots, so a removed or replaced source (or a changed key) answers null and the page offers Refresh. + * + * `createSourceRecovery` is the page's single flight: one rescope per held `(key, scope)` pair, its result shared + * by every caller (the inspector, the viewer and the Tag image dialog may all ask for the same file), and a pair + * already tried answers its first result, so a refusal can never loop. `apply(image, fresh, started)` is the page's + * part (refresh the collection, move the selection); `started` is the page generation when the rescope began, so a + * page whose filters changed meanwhile can leave itself alone. `reset()` (a manual Refresh) forgets every pair. + */ +import { imageKey } from './images-state.mjs'; + +export const RESCOPE_URL = '/prompt_manager/images/rescope'; +// A page holds a handful of stale files at once; older pairs are forgotten first. +const MEMO_LIMIT = 64; +const hex64 = value => typeof value === 'string' && /^[0-9a-f]{64}$/.test(value); + +function validTarget(value) { + return value !== null && typeof value === 'object' && Number.isSafeInteger(value.root_index) && value.root_index >= 0 && + hex64(value.root_scope) && typeof value.path === 'string' && value.path.length > 0; +} + +/** The fresh descriptor of `image` (same path, current root and scope), else null. */ +export async function rescopeImage(api, image, {signal} = {}) { + if (!validTarget(image)) return null; + let value; + try { + value = await api.post(RESCOPE_URL, {target: {root_index: image.root_index, root_scope: image.root_scope, path: image.path}}, + {signal}); + } catch (error) { + // 409 (the source is gone) or 404 (the file is gone): final, the caller offers Refresh. Anything else (no + // answer, a 5xx) is rethrown so the pair may be asked again. + if (error?.status === 404 || error?.status === 409) return null; + throw error; + } + const fresh = value?.success === true ? value.image : null; + if (!validTarget(fresh) || fresh.path !== image.path) return null; + return Object.freeze({...fresh}); +} + +export function createSourceRecovery({api, signal, apply, generation = () => 0}) { + // An embedded gallery's own page adapter may have no `post`: its rescope then simply finds nothing. + if (typeof apply !== 'function') throw new TypeError('createSourceRecovery needs apply'); + const memo = new Map(); + function recover(image) { + if (!validTarget(image)) return Promise.resolve(null); + const pair = `${imageKey(image)}|${image.root_scope}`; + if (memo.has(pair)) return memo.get(pair); + const started = generation(); + const pending = rescopeImage(api, image, {signal}) + .then(fresh => (fresh && !signal?.aborted ? apply(image, fresh, started) : null)) + .then(result => result ?? null, () => { + if (memo.get(pair) === pending) memo.delete(pair); + return null; + }); + memo.set(pair, pending); + if (memo.size > MEMO_LIMIT) memo.delete(memo.keys().next().value); + return pending; + } + return {recover, reset: () => memo.clear()}; +} + +/** + * The Images page's `apply` (#430). Keys are `root_index:path`, so after a renumbering nothing from the old list is + * kept by key (a checked or selected key could name a same-named file in another source): the viewer closes, the + * checks clear, a Source filter on the old index follows the file, and only the file itself stays selected while the + * collection lists again. An unchanged numbering re-reads the loaded range in place. `page` holds the page's hooks. + */ +export function recoveredSource(page) { + return async (image, fresh, started) => { + // The held scope still names its root: nothing is stale (a media error had another cause). + if (fresh.root_index === image.root_index && fresh.root_scope === image.root_scope) return null; + if (!page.current(started)) return fresh; + const selected = page.selected(image); + if (fresh.root_index === image.root_index) { + if (selected) page.inspect(fresh); + await page.stream.refresh().catch(() => {}); + return fresh; + } + page.closeViewer(); + page.clearChecks(); + const {filters} = page; + if (filters.root === String(image.root_index)) filters.root = String(fresh.root_index); + else if (filters.root !== 'all') { filters.root = 'all'; filters.subfolder = ''; } + if (selected) page.inspect(fresh); + else page.dropSelection(); + page.updateScope(); + page.stream.reset(filters, {load:page.listsNow()}); + page.rediscover(); + return fresh; + }; +} diff --git a/web/js/workspace/images-tagging.mjs b/web/js/workspace/images-tagging.mjs index dc3f0f91..2e5f36ba 100644 --- a/web/js/workspace/images-tagging.mjs +++ b/web/js/workspace/images-tagging.mjs @@ -14,9 +14,9 @@ export const REFRESH_FAILED = 'Images could not be refreshed after the tag save. /** Open Tag image for `image` when `action` is its card's Tag button (without selecting the card, as the concept's * `image-tag` does); false for any other control. */ -export function openCardTagging({api, scope, image, action, embedded}) { +export function openCardTagging({api, scope, image, action, embedded, onSourceChanged = null, onRefresh = null}) { if (!isTagButton(action)) return false; - openImageTagging({api, scope, image, source: embedded ? 'library' : 'images', trigger: action}); + openImageTagging({api, scope, image, source: embedded ? 'library' : 'images', trigger: action, onSourceChanged, onRefresh}); return true; } diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index bfa9348e..ad075fe6 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -21,6 +21,7 @@ import { displayPreferences } from './preferences.mjs'; import { onImageTagsSaved } from './image-tagging.mjs'; import { followImageTagSaves, openCardTagging } from './images-tagging.mjs'; import { favoriteBusy, paintStar, receiveFavorites, setFavoriteVisitTarget, toggleFavorite } from './images-favorites.mjs'; +import { createSourceRecovery, recoveredSource } from './images-source-recovery.mjs'; export { onFavoriteChange } from './images-favorites.mjs'; /** The concept's panel without an image (app.js updateImages a456–458). */ @@ -65,6 +66,7 @@ const IMAGE_SORT_OPTIONS = Object.freeze([['newest', 'Recently added'], ['name', const MEDIA_OPTIONS = Object.freeze([['all', 'All media'], ['image', 'Images'], ['video', 'Videos']]); const RETRY_RESCAN = 'Rescan from Import to retry.'; const RETRY_REFRESH = 'Refresh image collection to retry.'; +const CURSOR_STALE = 'The image collection changed. Refresh image collection to continue.'; /** One alert for whichever of the source discovery and the categories failed, so two failures announce once. */ function loadFailure(sourcesRetry, categoriesFailed) { if (sourcesRetry && categoriesFailed) return `Sources, folders and categories could not be loaded. ${sourcesRetry}`; @@ -122,7 +124,14 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage // System › Appearance: the display defaults this visit starts from (preferences.mjs); null without gallery-settings.js. const preferences = displayPreferences(); const inspection = createInspectionController({api, autoLoad:preferences?.autoLoadMetadata !== false, cache:preferences?.cacheMetadata !== false}); - scope.cleanup(inspection.subscribe(next => detailInspector?.setState(next))); + // The panel's listed file: a changed source is looked up once, the panel's Refresh stays the fallback (#430). + let selectedImage = null; + scope.cleanup(inspection.subscribe(next => { + detailInspector?.setState(next); + if (next.refreshRequired && next.code === 'source-changed' && selectedImage && imageKey(selectedImage) === selection) { + recovery.recover(selectedImage); + } + })); scope.cleanup(() => inspection.dispose()); scope.cleanup(() => detailInspector?.dispose()); // A Tags & categories Browse (browseImages) is handed to this mount once: a fresh Images visit filtered by its @@ -288,7 +297,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage main.append(body); const mediaViewer = createMediaViewer({api,container:main,parentScope:scope,inspectionController:inspection,sender, onSelect:image => {if (selection !== imageKey(image)) inspect(image);else inspection.select(image);}, - onRefresh:refreshInspectionCollection,onClose:scheduleLoad,onRestoreFocus:image => { + onRefresh:refreshInspectionCollection,onClose:scheduleLoad,onMediaError:image => recovery.recover(image),onRestoreFocus:image => { const index = state?.items.findIndex(entry => imageKey(entry) === imageKey(image)); if (index >= 0) focusImage(index);else focusFallback.focus({preventScroll:true}); }}); @@ -300,6 +309,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage function inspect(image) { detailInspector?.dispose(); selection = imageKey(image); + selectedImage = image; for (const video of details.querySelectorAll('video')) releaseVideo(video); detailInspector = mountFileInspector(details, {sender, parentScope:scope, variant:'panel', onTagFilter:embedded ? undefined : filterByTag, onRetry:() => inspection.retry(), onLoad:() => inspection.load(), onRefresh:refreshInspectionCollection}); @@ -340,7 +350,8 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage const action = event.target.closest('.image-card-actions button'); if (event.target.closest('.star-button')) toggleFavorite(api, image); else if (event.target.closest('.image-open')) previewImage(image); - else if (action && !openCardTagging({api, scope, image, action, embedded})) inspect(image); + else if (action && !openCardTagging({api, scope, image, action, embedded, onSourceChanged:recovery.recover, + onRefresh:refreshInspectionCollection})) inspect(image); }); function renderVisible() { @@ -480,6 +491,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage scope.cleanup(() => { resize.disconnect(); observer?.disconnect(); cancelAnimationFrame(resizeFrame); cancelAnimationFrame(frame); cancelAnimationFrame(loadFrame); }); function continuationText(next) { if (recovering) return 'Images paused until sources and folders are available.'; + if (next.error?.code === 'cursor-stale') return CURSOR_STALE; if (next.error) return 'Images could not be loaded. Retry or rescan the collection.'; if (next.loading && !next.loaded) return 'Loading images…'; if (next.hasMore) return `${next.loaded.toLocaleString('en-US')} of ${next.total.toLocaleString('en-US')} ${noun(next.total, 'image')} · more load as you scroll`; @@ -516,6 +528,8 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage setText(message, continuationText(next)); setAttr(message, 'role', next.error ? 'alert' : 'status'); retry.hidden = !next.error; + // A cursor the server can no longer verify is never resent: Retry becomes the collection refresh (#430). + setText(retry, next.error?.code === 'cursor-stale' ? 'Refresh image collection' : 'Retry'); more.hidden = !next.hasMore || !!next.error; more.setAttribute('aria-disabled', String(next.loading || recovering)); if (showsEmpty) empty.hidden = recovering || next.loading || !!next.error || next.hasMore || next.loaded > 0; @@ -553,7 +567,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage } } scope.listen(more, 'click', pressLoad); - scope.listen(retry, 'click', pressLoad); + scope.listen(retry, 'click', () => (state?.error?.code === 'cursor-stale' ? refreshInspectionCollection() : pressLoad())); function maybeLoad() { if (!observer || recovering || scope.signal.aborted || !state?.hasMore || state.loading || state.error || main.querySelector('#viewer-dialog[open]')) return; const bounds = continuation.getBoundingClientRect(); @@ -581,8 +595,18 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage details.replaceChildren(noSelection()); } selection = null; + selectedImage = null; } + // Bumped by every reset: a source recovery that began before it leaves the page alone (#430). + let generation = 0; + const recovery = createSourceRecovery({api, signal:scope.signal, generation:() => generation, apply:recoveredSource({ + selected:image => selection === imageKey(image), inspect, dropSelection, stream, filters, updateScope, + closeViewer:() => { mediaViewer.stopSlideshow(); main.querySelector('#viewer-dialog[open]')?.close(); }, + clearChecks:() => bar?.clear(), listsNow:() => !recovering, + current:started => !scope.signal.aborted && started === generation, + rediscover:() => loadOptions().then(found => { if (found) renderLoadStatus(); })})}); function reset() { + generation += 1; mediaViewer.stopSlideshow(); main.querySelector('#viewer-dialog[open]')?.close(); dropSelection({keepPanel:recovering}); @@ -686,6 +710,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage } async function refreshInspectionCollection() { if (scope.signal.aborted) return; + recovery.reset(); // Numeric roots may now name another source; retire the old stream first. recovering = true; filters.root = 'all';filters.subfolder = ''; diff --git a/web/js/workspace/inspection-state.mjs b/web/js/workspace/inspection-state.mjs index 44639bce..ef9050e8 100644 --- a/web/js/workspace/inspection-state.mjs +++ b/web/js/workspace/inspection-state.mjs @@ -21,6 +21,7 @@ export class InspectionSourceChangedError extends Error { super(SOURCE_CHANGED); this.name = 'InspectionSourceChangedError'; this.refreshRequired = true; + this.code = 'source-changed'; } } diff --git a/web/js/workspace/media-viewer.mjs b/web/js/workspace/media-viewer.mjs index 834c9957..106932af 100644 --- a/web/js/workspace/media-viewer.mjs +++ b/web/js/workspace/media-viewer.mjs @@ -19,7 +19,7 @@ function element(tag, className, text) { return node; } -export function createMediaViewer({api,container,parentScope,inspectionController,sender,onSelect,onRefresh,onRestoreFocus,onClose,adapter = rootedFileAdapter}) { +export function createMediaViewer({api,container,parentScope,inspectionController,sender,onSelect,onRefresh,onRestoreFocus,onClose,onMediaError,adapter = rootedFileAdapter}) { const scope = createScope(); let active = null, retryVersion = 0; const dispose = () => scope.dispose(); @@ -148,7 +148,8 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle src = url.href; } const node = element(selected.is_video ? 'video' : 'img'); - mediaLocal.listen(node,'error',() => {if (current()) session.failed(key);}); + // #430: the page may find the file under a renumbered source (`onMediaError`). + mediaLocal.listen(node,'error',() => {if (current()) {session.failed(key);onMediaError?.(selected);}}); if (selected.is_video) { node.controls = true;node.playsInline = true;node.preload = 'metadata'; const preferences = readViewerPreferences(); diff --git a/web/js/workspace/workspace.css b/web/js/workspace/workspace.css index 8cf2720b..386f7b40 100644 --- a/web/js/workspace/workspace.css +++ b/web/js/workspace/workspace.css @@ -450,3 +450,9 @@ pre.prompt-text { font: inherit; font-size: 12px; line-height: 1.75; margin-bloc /* #332 D146: the category chip names its origin (" · this image", " · prompt", " · recipe") in muted text that never splits; at 390px the chip may wrap between the category and its origin. */ .chip .chip-source { color: var(--ca-muted); white-space: nowrap; } +/* D147 (#430): a state that blocks Save shows its reason on its own row above the sticky actions, with Refresh for a + changed source; the disabled actions look disabled; a refused capture's empty preview says so. */ +.tagging-modal .dialog-actions [aria-disabled="true"], .tagging-modal .tag-model-panel [aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* D147 */ } +.tagging-modal .tag-action-notice { flex: 1 0 100%; display: flex; flex-wrap: wrap; align-items: center; justify-content: space-between; gap: 8px; margin: 0; color: var(--ca-text); font-size: 12px; /* D147 */ } +.tagging-modal .tag-action-notice > span { flex: 1 1 16em; } +.tag-image-preview.tag-preview-empty { display: grid; place-items: center; padding: 8px; text-align: center; font-size: 12px; color: var(--ca-muted); background: var(--ca-field); box-sizing: border-box; /* D147 */ }