From f9835aa4d66f746c40c30b2f00668b82d6e2b549 Mon Sep 17 00:00:00 2001 From: Vito Date: Sun, 25 Oct 2026 12:53:26 -0700 Subject: [PATCH 1/2] fix(lora): allow civitai.red/.green and log why a CivitAI download failed (#340) (#341) civitai.red and civitai.green (and their subdomains) are now allowed redirect targets under the civitai.com rules: HTTPS only, default port, no credentials, matched on the parsed host name, at most 3 hops, never with the key. Their www/api/image hosts are also allowed starting hosts, whose first request carries the key as image.civitai.com does. Each failed download now logs one warning with the URL without query or credentials and a short reason: HTTP status, refused redirect, hop limit, size cap, decode or save error, timeout or network error. --- README.md | 2 +- TESTING.md | 9 +- py/lora_utils.py | 140 +++++++++++------ tests/test_lora_utils.py | 316 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 415 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 7662730c..5ff18607 100644 --- a/README.md +++ b/README.md @@ -501,7 +501,7 @@ To add your key: 2. Paste it into the **CivitAI API Key** field in the Integrations settings 3. Save and re-import to download any previously skipped images -The key is sent only over HTTPS to CivitAI's own hosts. CivitAI serves images through a redirect to its storage host; PromptManager follows such a redirect only over HTTPS to a `civitai.com` host, at most three hops, and never sends the key with it. When downloads fail, the import's status line says how many; when none could be downloaded, it starts with a warning. +The key is sent only over HTTPS to CivitAI's own hosts. CivitAI serves images through a redirect to its storage host; PromptManager follows such a redirect only over HTTPS to a host on CivitAI's own `civitai.com`, `civitai.red` or `civitai.green` domains, at most three hops, and never sends the key with it. When downloads fail, the import's status line says how many; when none could be downloaded, it starts with a warning. The server log has one warning per failed image with the reason (HTTP status, refused redirect, size cap, unreadable image, timeout or network error). > **Note:** Re-importing is safe — PromptManager skips LoRAs that were already imported. To do a fresh import, the previous `lora-manager` data is cleared automatically before re-scanning. diff --git a/TESTING.md b/TESTING.md index 898cba82..8ea2d7f1 100644 --- a/TESTING.md +++ b/TESTING.md @@ -3151,7 +3151,14 @@ replace binds the category, not one parameter per prompt, so SQLite's variable l never caps a collection. Cached CivitAI images are written to a `.pm-*.part` file in the same folder and renamed into place, with the umask's mode as a plain save gives; each import first removes `.pm-*.part` files older than an hour from the image cache and its LoRA -folders (`TestStaleDownloadSweep`). The legacy `POST /lora/scan` runs the same stages under its SSE +folders (`TestStaleDownloadSweep`). A download starts only at an HTTPS `civitai.com`, +`civitai.red` or `civitai.green` host from the fixed `CIVITAI_HOSTS` list (the key goes +with that first request only); a redirect is followed only over HTTPS to one of those +domains or a subdomain (parsed host name, default port, no credentials), at most three +hops, without the key (`TestCivitaiRedirects`, `TestCivitaiRedAndGreenDomains`). Each +failed download logs one warning naming the URL without its query and a short reason +(HTTP status, refused redirect, hop limit, size cap, decode or save error, timeout or +network error), never the key (`TestDownloadFailureWarnings`). The legacy `POST /lora/scan` runs the same stages under its SSE event shape; a page that closes its connection stops staging before its next LoRA, and the scan keeps its records hold until staging has stopped, then ends `cancelled` in Activity (`test_a_disconnected_legacy_scan_stops_and_holds_until_it_stopped`) (`test_legacy_scan_events_unchanged_on_success`), holds diff --git a/py/lora_utils.py b/py/lora_utils.py index ef77b861..5965af49 100644 --- a/py/lora_utils.py +++ b/py/lora_utils.py @@ -8,6 +8,7 @@ return empty results rather than raising. """ import hashlib +import http.client import json import os import re @@ -420,22 +421,35 @@ def get_preview_image_from_metadata( _THUMB_MAX_SIZE = 512 +# CivitAI's own domains (#340: civitai.red and civitai.green serve the same +# content as civitai.com). +CIVITAI_DOMAINS = ("civitai.com", "civitai.red", "civitai.green") # Only these hosts may ever receive the CivitAI API key, and only over HTTPS. CIVITAI_HOSTS = frozenset( - {"civitai.com", "www.civitai.com", "api.civitai.com", "image.civitai.com"} + f"{prefix}{domain}" + for domain in CIVITAI_DOMAINS + for prefix in ("", "www.", "api.", "image.") ) MAX_CIVITAI_DOWNLOAD_BYTES = 50 * 1024 * 1024 _DOWNLOAD_CHUNK_BYTES = 64 * 1024 # A redirect is followed (#337: image.civitai.com now answers with a redirect -# to blobs-b2.civitai.com) only over HTTPS to civitai.com or a subdomain of -# it, at most this many hops, and never with the API key. +# to blobs-b2.civitai.com) only over HTTPS to one of CIVITAI_DOMAINS or a +# subdomain of one, at most this many hops, and never with the API key. MAX_CIVITAI_REDIRECTS = 3 _REDIRECT_CODES = frozenset({301, 302, 303, 307, 308}) _USER_AGENT = "ComfyUI-PromptManager/1.0" +class _DownloadFailed(Exception): + """A download failed for the short, loggable ``reason`` it carries.""" + + def __init__(self, reason: str): + super().__init__(reason) + self.reason = reason + + class _RefuseRedirects(urllib.request.HTTPRedirectHandler): """Never follow a redirect inside urllib: it would re-send the Bearer header. A 3xx surfaces as ``HTTPError``; ``_download_one`` decides.""" @@ -467,10 +481,18 @@ def _loggable(url: str) -> str: return f"{parts.scheme}://{host}{parts.path}" +def _on_civitai_domain(hostname: str) -> bool: + """True when ``hostname`` is one of CIVITAI_DOMAINS or a subdomain of one.""" + return any( + hostname == domain or hostname.endswith(f".{domain}") + for domain in CIVITAI_DOMAINS + ) + + def is_civitai_redirect_target(url: str) -> bool: - """True when a redirect to ``url`` may be followed: HTTPS on civitai.com - or a subdomain of it, matched on the parsed host name, default port and - no credentials in the URL.""" + """True when a redirect to ``url`` may be followed: HTTPS on one of + CIVITAI_DOMAINS or a subdomain of one, matched on the parsed host name, + default port and no credentials in the URL.""" try: parts = urlsplit(url) port = parts.port @@ -479,7 +501,7 @@ def is_civitai_redirect_target(url: str) -> bool: hostname = (parts.hostname or "").lower() return ( parts.scheme == "https" - and (hostname == "civitai.com" or hostname.endswith(".civitai.com")) + and _on_civitai_domain(hostname) and parts.username is None and parts.password is None and port in (None, 443) @@ -500,8 +522,9 @@ def _open_following_civitai_redirects(url: str, api_key: str): """Open ``url``, following only allowed CivitAI redirects (keyless). The first request carries the key; every redirected one carries the - User-Agent alone. Returns the open response, or None when a redirect is - refused or the hop limit is reached. Other HTTP errors propagate. + User-Agent alone. Returns the open response; raises ``_DownloadFailed`` + when a redirect is refused or the hop limit is reached. Other HTTP + errors propagate. """ headers = {"User-Agent": _USER_AGENT} if api_key: @@ -517,20 +540,13 @@ def _open_following_civitai_redirects(url: str, api_key: str): if target is None: raise if hop == MAX_CIVITAI_REDIRECTS: - logger.warning( - f"Refusing redirect past {MAX_CIVITAI_REDIRECTS} hops " - f"from {_loggable(url)}" - ) - return None + raise _DownloadFailed(f"more than {MAX_CIVITAI_REDIRECTS} redirects") if not is_civitai_redirect_target(target): - logger.warning( - f"Refusing redirect from {_loggable(current)} to {_loggable(target)}" - ) - return None + raise _DownloadFailed(f"redirect refused to {_loggable(target)}") logger.debug(f"Following redirect to {_loggable(target)}") current = target headers = {"User-Agent": _USER_AGENT} # never the key across a redirect - return None # pragma: no cover - the loop returns on its last hop + raise AssertionError("unreachable") # pragma: no cover - loop ends above def is_civitai_url(url: str) -> bool: @@ -573,40 +589,76 @@ def _read_capped(resp, cap: int) -> Optional[bytes]: return b"".join(chunks) +def _network_reason(error: BaseException) -> str: + """A short reason for a failed request: a timeout or a network error.""" + cause = getattr(error, "reason", error) # URLError wraps the socket error + if isinstance(cause, TimeoutError): + return "timed out" + return f"network error: {cause}" + + +def _fetch_civitai_body(url: str, api_key: str) -> bytes: + """The body behind ``url`` (redirects as allowed, size capped), or + ``_DownloadFailed`` with the reason it could not be fetched.""" + try: + with _open_following_civitai_redirects(url, api_key) as resp: + raw = _read_capped(resp, MAX_CIVITAI_DOWNLOAD_BYTES) + except urllib.error.HTTPError as error: + error.close() + raise _DownloadFailed(f"HTTP {error.code}") from None + except (urllib.error.URLError, OSError, http.client.HTTPException) as error: + raise _DownloadFailed(_network_reason(error)) from None + if raw is None: + cap = MAX_CIVITAI_DOWNLOAD_BYTES + size = f"{cap // (1024 * 1024)} MB" if cap >= 1024 * 1024 else f"{cap} byte" + raise _DownloadFailed(f"larger than the {size} cap") + return raw + + +def _thumbnail(raw: bytes): + """``raw`` decoded and shrunk to an RGB thumbnail, or ``_DownloadFailed``.""" + from io import BytesIO + + from PIL import Image + + try: + with Image.open(BytesIO(raw)) as img: + img.thumbnail((_THUMB_MAX_SIZE, _THUMB_MAX_SIZE), Image.LANCZOS) + return img.convert("RGB") + except Exception as error: # PIL raises several types for bad data + raise _DownloadFailed( + f"image could not be decoded ({type(error).__name__})" + ) from None + + def _download_one(url: str, local_path: Path, api_key: str) -> Optional[str]: """Download a single CivitAI image, resize to thumbnail, save as JPEG. Refuses any URL that is not HTTPS on an allow-listed CivitAI host so the API key is never sent elsewhere, follows a redirect only as ``_open_following_civitai_redirects`` allows (without the key), and - refuses bodies larger than MAX_CIVITAI_DOWNLOAD_BYTES. + refuses bodies larger than MAX_CIVITAI_DOWNLOAD_BYTES. A failure logs + one warning with a short reason and the URL without query or + credentials (#340), and returns None. """ - if not is_civitai_url(url): - logger.warning(f"Refusing non-CivitAI download URL: {_loggable(url)}") - return None try: - resp = _open_following_civitai_redirects(url, api_key) - if resp is None: - return None - with resp: - raw = _read_capped(resp, MAX_CIVITAI_DOWNLOAD_BYTES) - if raw is None: - logger.warning(f"Refusing oversized download: {_loggable(url)}") - return None - - # Resize to thumbnail to save disk space - from io import BytesIO - - from PIL import Image - - with Image.open(BytesIO(raw)) as img: - img.thumbnail((_THUMB_MAX_SIZE, _THUMB_MAX_SIZE), Image.LANCZOS) - rgb = img.convert("RGB") - _save_atomically(rgb, local_path) + if not is_civitai_url(url): + raise _DownloadFailed("not an allowed CivitAI URL") + image = _thumbnail(_fetch_civitai_body(url, api_key)) + try: + _save_atomically(image, local_path) + except OSError as error: + raise _DownloadFailed( + f"image could not be saved ({error.strerror or type(error).__name__})" + ) from None + except _DownloadFailed as failure: + reason = failure.reason + except Exception as error: # best effort: an import never fails on one image + reason = f"unexpected {type(error).__name__}" + else: return str(local_path.resolve()) - except Exception as e: - logger.debug(f"Failed to download {_loggable(url)}: {e}") - return None + logger.warning(f"CivitAI image download failed: {_loggable(url)}: {reason}") + return None _PART_PREFIX = ".pm-" diff --git a/tests/test_lora_utils.py b/tests/test_lora_utils.py index 5de87d2d..7348c83f 100644 --- a/tests/test_lora_utils.py +++ b/tests/test_lora_utils.py @@ -296,12 +296,9 @@ class TestCivitaiUrlPolicy(unittest.TestCase): self.assertEqual( CIVITAI_HOSTS, frozenset( - { - "civitai.com", - "www.civitai.com", - "api.civitai.com", - "image.civitai.com", - } + f"{prefix}{domain}" + for domain in ("civitai.com", "civitai.red", "civitai.green") + for prefix in ("", "www.", "api.", "image.") ), ) self.assertEqual(MAX_CIVITAI_DOWNLOAD_BYTES, 50 * 1024 * 1024) @@ -334,6 +331,36 @@ class TestCivitaiUrlPolicy(unittest.TestCase): def test_explicit_default_port_is_allowed(self): self.assertTrue(is_civitai_url("https://civitai.com:443/x")) + def test_red_and_green_starting_hosts_are_allowed(self): + # #340: CivitAI's own .red and .green domains serve the same content. + for url in ( + "https://image.civitai.red/xG1/abc/width=450/1.jpeg", + "https://civitai.red/api/download/1", + "https://civitai.green/api/download/1", + "https://image.civitai.green/xG1/abc/1.jpeg", + "https://IMAGE.CIVITAI.RED/x", + "https://civitai.green:443/x", + ): + self.assertTrue(is_civitai_url(url), url) + + def test_red_and_green_lookalikes_are_refused(self): + for url in ( + "http://image.civitai.red/x", + "http://civitai.green/x", + "https://civitai.red.evil.example/x", + "https://civitai.green.evil.example/x", + "https://evilcivitai.red/x", + "https://evilcivitai.green/x", + "https://civitai.redx/x", + "https://civitai.greenx/x", + "https://civitai.red@evil.example/x", + "https://user:pw@civitai.red/x", + "https://civitai.green:8443/x", + "https://blobs-b2.civitai.red/x", # starting hosts stay a fixed list + "https://evil.example/civitai.red", + ): + self.assertFalse(is_civitai_url(url), url) + class TestLoraManagerPathPolicy(unittest.TestCase): """Only a direct child of a ComfyUI custom_nodes dir named *lora* qualifies.""" @@ -891,6 +918,283 @@ class TestCivitaiRedirects(unittest.TestCase): self.assertNotIn("sig123", text) +RED_BLOB = "https://blobs-b2.civitai.red/file/civitai-media-cache/1/original" +GREEN_BLOB = "https://media.civitai.green/file/1/original" + + +class TestCivitaiRedAndGreenDomains(unittest.TestCase): + """#340: civitai.red and civitai.green are CivitAI's own domains. + + They are redirect targets under the civitai.com rules (HTTPS, default + port, no credentials, parsed host name, 3 hops, never the key) and + starting hosts whose first request may carry the key. + """ + + def setUp(self): + self.tmpdir = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.tmpdir, True) + self.target = self.tmpdir / "x.jpg" + + def _download(self, routes, url=START, api_key="secret-key"): + handler = _ChainHTTPSHandler(routes) + opener = lora_utils._build_opener(handler) + with patch.object(lora_utils, "_opener", opener): + result = lora_utils._download_one(url, self.target, api_key) + return result, handler.requests + + def test_redirect_targets_on_red_and_green_are_allowed(self): + for url in ( + "https://civitai.red/x", + "https://civitai.green/x", + RED_BLOB, + GREEN_BLOB, + "https://a.b.civitai.red:443/x", + "https://CIVITAI.GREEN/x", + ): + self.assertTrue(lora_utils.is_civitai_redirect_target(url), url) + + def test_redirect_target_lookalikes_are_refused(self): + for url in ( + "https://civitai.red.evil.example/x", + "https://civitai.green.evil.example/x", + "https://evilcivitai.red/x", + "https://evilcivitai.green/x", + "https://civitai.redx/x", + "https://civitai.greenx/x", + "https://civitai.red@evil.example/x", + "https://user:pw@blobs-b2.civitai.red/x", + "https://user@civitai.green/x", + "https://blobs-b2.civitai.red:8443/x", + "http://blobs-b2.civitai.red/x", + "http://civitai.green/x", + "ftp://civitai.red/x", + "https://evil.example/civitai.red", + "https://evil.example/?h=civitai.green", + "https://red/x", + "https://.civitai.red.evil/x", + ): + self.assertFalse(lora_utils.is_civitai_redirect_target(url), url) + + def test_a_redirect_to_red_or_green_is_followed_without_the_key(self): + for blob in (RED_BLOB, GREEN_BLOB): + with self.subTest(blob=blob): + if self.target.exists(): + os.remove(self.target) + result, requests = self._download( + {START: (302, blob), blob: (200, None)} + ) + + self.assertIsNotNone(result) + self.assertEqual([r.full_url for r in requests], [START, blob]) + self.assertEqual( + requests[0].get_header("Authorization"), "Bearer secret-key" + ) + self.assertIsNone(requests[1].get_header("Authorization")) + + def test_lookalike_and_plain_http_redirects_are_refused(self): + for location in ( + "https://civitai.red.evil.example/x", + "https://evilcivitai.red/x", + "https://civitai.redx/x", + "https://civitai.green.evil.example/x", + "https://evilcivitai.green/x", + "https://civitai.greenx/x", + "http://blobs-b2.civitai.red/x", + "http://civitai.green/x", + ): + with self.subTest(location=location): + result, requests = self._download({START: (302, location)}) + self.assertIsNone(result) + self.assertEqual([r.full_url for r in requests], [START]) + self.assertFalse(self.target.exists()) + + def test_a_red_starting_host_gets_the_key_and_its_redirect_does_not(self): + start = "https://image.civitai.red/xG1/abc/width=450/1.jpeg" + result, requests = self._download( + {start: (302, BLOB), BLOB: (200, None)}, url=start + ) + + self.assertIsNotNone(result) + self.assertEqual([r.full_url for r in requests], [start, BLOB]) + self.assertEqual(requests[0].get_header("Authorization"), "Bearer secret-key") + self.assertIsNone(requests[1].get_header("Authorization")) + + def test_a_green_starting_host_downloads_directly(self): + start = "https://civitai.green/api/download/1" + result, requests = self._download({start: (200, None)}, url=start) + + self.assertIsNotNone(result) + self.assertEqual(requests[0].get_header("Authorization"), "Bearer secret-key") + + def test_the_hop_limit_spans_domains(self): + hops = [RED_BLOB, GREEN_BLOB, BLOB, "https://civitai.red/4"] + routes = {START: (302, hops[0])} + for here, there in zip(hops, hops[1:]): + routes[here] = (302, there) + routes[hops[-1]] = (200, None) + + result, requests = self._download(routes) + + self.assertIsNone(result) + self.assertEqual(len(requests), lora_utils.MAX_CIVITAI_REDIRECTS + 1) + self.assertNotIn(hops[-1], [r.full_url for r in requests]) + for req in requests[1:]: + self.assertIsNone(req.get_header("Authorization")) + + +class _RaisingOpener: + """An ``_open_url`` stand-in that raises ``error`` for every request.""" + + def __init__(self, error): + self.error = error + self.requests = [] + + def __call__(self, req, timeout=None): + self.requests.append(req) + raise self.error + + +class TestDownloadFailureWarnings(unittest.TestCase): + """#340: each failed download logs one warning line with a short reason, + the URL without its query or credentials, and never the key.""" + + LOGGER = "prompt_manager.lora_utils" + SIGNED = START + "?token=t0k3n" + + def setUp(self): + self.tmpdir = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.tmpdir, True) + self.target = self.tmpdir / "x.jpg" + + def _warning(self, run): + """Run ``run`` and return its one WARNING-or-above record's text.""" + with self.assertLogs(self.LOGGER, level="WARNING") as logs: + result = run() + self.assertIsNone(result) + self.assertFalse(self.target.exists()) + self.assertEqual(len(logs.records), 1, logs.output) + record = logs.records[0] + self.assertEqual(record.levelname, "WARNING") + text = record.getMessage() + self.assertNotIn("secret-key", text) + self.assertNotIn("t0k3n", text) + self.assertNotIn("sig123", text) + self.assertIn(START.split("?")[0], text) + return text + + def _chain(self, routes, url=SIGNED): + handler = _ChainHTTPSHandler(routes) + opener = lora_utils._build_opener(handler) + + def run(): + with patch.object(lora_utils, "_opener", opener): + return lora_utils._download_one(url, self.target, "secret-key") + + return run + + def _fake(self, opener, url=SIGNED): + def run(): + with patch.object(lora_utils, "_open_url", opener): + return lora_utils._download_one(url, self.target, "secret-key") + + return run + + def test_http_status(self): + text = self._warning(self._chain({})) # unknown URL answers 404 + self.assertIn("HTTP 404", text) + + def test_http_status_after_a_redirect(self): + text = self._warning(self._chain({self.SIGNED: (302, BLOB)})) + self.assertIn("HTTP 404", text) + + def test_refused_redirect(self): + text = self._warning( + self._chain({self.SIGNED: (302, "https://evil.example/x?sig123=1")}) + ) + self.assertIn("redirect refused", text) + self.assertIn("https://evil.example/x", text) + + def test_hop_limit(self): + text = self._warning(self._chain({self.SIGNED: (302, self.SIGNED)})) + self.assertIn("more than 3 redirects", text) + + def test_size_cap(self): + opener = _FakeOpener( + _FakeResponse(_png_bytes(), content_length=MAX_CIVITAI_DOWNLOAD_BYTES + 1) + ) + text = self._warning(self._fake(opener)) + self.assertIn("larger than the 50 MB cap", text) + + def test_decode_error(self): + text = self._warning(self._fake(_FakeOpener(_FakeResponse(b"not an image")))) + self.assertIn("image could not be decoded", text) + + def test_timeout(self): + for error in ( + TimeoutError("timed out"), + urllib.error.URLError(TimeoutError("timed out")), + ): + with self.subTest(error=type(error).__name__): + text = self._warning(self._fake(_RaisingOpener(error))) + self.assertIn("timed out", text) + + def test_network_error(self): + for error in ( + urllib.error.URLError("[Errno -2] Name or service not known"), + ConnectionResetError(104, "Connection reset by peer"), + ): + with self.subTest(error=type(error).__name__): + text = self._warning(self._fake(_RaisingOpener(error))) + self.assertIn("network error", text) + + def test_a_timeout_while_reading_the_body(self): + class SlowResponse(_FakeResponse): + def read(self, amt=-1): + raise TimeoutError("timed out") + + text = self._warning(self._fake(_FakeOpener(SlowResponse(b"")))) + self.assertIn("timed out", text) + + def test_a_failed_save(self): + opener = _FakeOpener(_FakeResponse(_png_bytes())) + + def run(): + with patch.object( + lora_utils.os, "replace", side_effect=OSError(28, "No space left") + ): + return self._fake(opener)() + + text = self._warning(run) + self.assertIn("could not be saved", text) + + def test_a_refused_starting_url(self): + def run(): + return lora_utils._download_one( + "http://image.civitai.com/x?token=t0k3n", self.target, "secret-key" + ) + + with self.assertLogs(self.LOGGER, level="WARNING") as logs: + self.assertIsNone(run()) + self.assertEqual(len(logs.records), 1) + text = logs.records[0].getMessage() + self.assertIn("not an allowed CivitAI URL", text) + self.assertNotIn("t0k3n", text) + + def test_an_unexpected_error_names_only_its_type(self): + error = ValueError("unknown url type: https://x?token=t0k3n") + text = self._warning(self._fake(_RaisingOpener(error))) + self.assertIn("unexpected ValueError", text) + + def test_the_line_names_the_download(self): + text = self._warning(self._chain({})) + self.assertTrue(text.startswith("CivitAI image download failed"), text) + + def test_a_successful_download_logs_no_warning(self): + with self.assertNoLogs(self.LOGGER, level="WARNING"): + result = self._fake(_FakeOpener(_FakeResponse(_png_bytes())))() + self.assertIsNotNone(result) + + class TestDownloadCivitaiImages(unittest.TestCase): def setUp(self): From fb9134f081c4abccd90651bc2348f6b7813f5506 Mon Sep 17 00:00:00 2001 From: Vito Date: Sun, 25 Oct 2026 15:32:50 -0700 Subject: [PATCH 2/2] feat(system): Backup & export (System S9) (#339) * feat(system): back up, restore and export natively System > Backup & export replaces its hand-off with the concept's section on the S8 backend: Back up downloads the verified copy and the status line shows only what GET /backup/status reports (D103); Review restore stages a chosen file and shows the server's review in the shared review dialog, Cancel discards the stage and Restore runs the Database restore job, after which the open views reload (D105); Export JSON plus the components and saved-pairs exports (D103); Import / inspect opens the shared Import & inspect dialog; the safety-backup policy saves through the Save row and scheduled backups offer Manual only (D104). Overview's Backup status row reads the same status. Refs #326 * test(system): compare Backup & export with the concept system-backups-concept.spec compares the section, the Restore preview and the Import & inspect dialog with the concept in both themes at 1600 and 390 wide, mapping only D103, D104, D105 (and D15/D55 for the shared Import dialog), and proves a renamed label, a removed row, a lost danger mark and swapped tokens are noticed. A review's grid values now wrap anywhere (D105), so a long backup file name keeps the concept's two equal columns at phone width. Refs #326 * docs(testing): Backup & export contract (D103-D105) Refs #326 * test(coverage): raise the frontend floors S9 measured system-backups.mjs gets its measured floor in place of the provisional one; admin.js, activity-scan.mjs and maintenance-review.mjs branches go up. Refs #326 * fix(system): keep a refused or handed-back restore review usable A Restore refused as busy now opens the same review again with Discard / Restore, and a restore job whose leases did not drain hands its review back (scope.review_kept) to the page that started it, which reopens it; every other path that abandons a token after Restore sends its DELETE, so held reviews can no longer lock restore out. The kept-review line says where the review is, and a full review store says what to do (I1). applyPlan resolves with its outcome and its job tracking now has failing-before proof (N5). Also: the safety-backup toggle says what the policy does (D104, M2); the restore picker has no type filter so safety backups can be chosen (M3); "Export all prompts JSON" names what /export holds (N1); small snapshots show KB (N3); the duplicated holding and save helpers are shared (N4); a dismissal test covers Cancel, x, Escape, a replacing dialog and leaving (M1). Refs #326 * test(coverage): raise the system-backups branch floor Refs #326 --- TESTING.md | 103 +++- py/api/backup_restore.py | 7 +- tests/browser/system-backups-concept.spec.mjs | 329 ++++++++++++ tests/browser/system-concept.spec.mjs | 7 +- .../browser/workspace-system-backups.spec.mjs | 489 ++++++++++++++++++ tests/browser/workspace-system.spec.mjs | 5 +- tests/browser/workspace.spec.mjs | 10 +- tests/browser/workspace_system_fixture.py | 40 ++ tests/js/workspace-activity-kinds.test.js | 2 +- tests/js/workspace-activity-store.test.js | 2 +- tests/js/workspace-system-backups.test.js | 286 ++++++++++ tests/js/workspace-system-overview.test.js | 11 +- tests/js/workspace-toolbar.test.js | 36 +- tests/test_restore_review_fixes.py | 28 +- tools/vendor/frontend-coverage-baseline.json | 12 +- web/js/workspace/activity-kinds.mjs | 13 +- web/js/workspace/activity-scan.mjs | 16 +- web/js/workspace/images.mjs | 15 +- web/js/workspace/index.mjs | 21 +- web/js/workspace/library.mjs | 4 + web/js/workspace/maintenance-review.mjs | 28 +- web/js/workspace/system-backups.mjs | 464 +++++++++++++++++ web/js/workspace/system-overview.mjs | 40 +- web/js/workspace/system.mjs | 9 +- web/js/workspace/workspace-dialogs.mjs | 25 +- web/js/workspace/workspace.css | 3 + 26 files changed, 1935 insertions(+), 70 deletions(-) create mode 100644 tests/browser/system-backups-concept.spec.mjs create mode 100644 tests/browser/workspace-system-backups.spec.mjs create mode 100644 tests/js/workspace-system-backups.test.js create mode 100644 web/js/workspace/system-backups.mjs diff --git a/TESTING.md b/TESTING.md index 8ea2d7f1..9149ba93 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2378,7 +2378,8 @@ paints nothing. Until it answers, the section shows "Loading the overview…"; a shows "The overview could not be loaded. Your saved data has not changed." and Retry, never zeros. The thumbnail row is a separate read of `/images/thumbnails/status` (a walk of every source), so the other values never wait for it: "Checking thumbnails…" -until it answers. The health check (`POST /prompt_manager/system/health`) is a `read` +until it answers. The Backup status row is likewise its own read of +`/prompt_manager/backup/status` (S9). The health check (`POST /prompt_manager/system/health`) is a `read` registry job of kind `health`: one at a time, beside a scan, not cancellable and with no file outcomes. The page tracks the started job in the Activity store (its event stream; the store marks it lost if the server stops listing it), never by polling. A press while one runs (another tab) waits @@ -2399,7 +2400,7 @@ the connection's `total_changes` is 0 at close and the data reads back unchanged `-shm` bytes are not compared: every WAL reader rewrites its read marks there, so no reader can keep it byte-identical (`test_health_check_never_writes`). -Recorded deviations (plan D60–D70, D74, D75, D79–D82 renumbered D92–D102, D106, D107 and D111–D114; D121–D131 assigned): +Recorded deviations (plan D60–D75, D79–D82 renumbered D92–D107 and D111–D114; D121–D131 assigned): - **D92** (shell): no "Sample configuration" chip. The Save status is truthful: "No unsaved changes.", "Unsaved changes.", "Saved in this browser." / "Saved to the server @@ -2416,7 +2417,10 @@ Recorded deviations (plan D60–D70, D74, D75, D79–D82 renumbered D92–D102, `needs_review.pending`) with the concept's line; its Review metadata action opens `#/review` (the concept's `go-review`). A count that is not an integer reads "Ambiguous image links" / "Not available yet." (Q6). - "Backup status" reads "No retained backup record." until verified backups exist (S8). + "Backup status" says what `GET /prompt_manager/backup/status` reports, in Backup & + export's own status line (D103, S9): "Checking backups…" until its own read answers, + then the session's last download and the newest safety backup, "No backup recorded." + or "The backup status could not be read.". The thumbnail row is "Checking thumbnails…" until its own read answers, then "No missing thumbnails", "N missing thumbnails" (the pending count of `/images/thumbnails/status`, followed by " · M outdated" when previews are in an @@ -2615,6 +2619,68 @@ Recorded deviations (plan D60–D70, D74, D75, D79–D82 renumbered D92–D102, is D93's style. `maintenance matches the concept` (both themes and sizes) maps only this row; `maintenance comparison detects mutations` proves it is the only difference and catches a removed row, a renamed operation, a lost danger mark and a swapped token. +- **D103** (Backup & export, S9): "Back up" (the concept's "Simulate backup") downloads + the verified copy of `GET /prompt_manager/backup` through the api client (the same + `downloadDatabaseBackup` as every backup box; a copy whose `X-PromptManager-Verify` is + not `ok` toasts "Database backup downloaded, but it did not pass verification: + "), then reads the status again. The `p.small#backup-status` under the + actions shows only what `GET /prompt_manager/backup/status` reports: "Downloaded + backup · · verified." (or "not verified") for this session's last + download and "Safety backup · · N on disk." for the newest + `.backup_*`, else "No backup recorded."; "Checking backups…" while read and "The + backup status could not be read." when it failed (never "no backup"). Portable + export's "Export JSON" (the concept's "Export sample JSON") downloads the Library + document (`/library/export`); a second `.row-actions.spaced` adds "Export all prompts + JSON" (`/export`, the legacy admin page's Export of every prompt) and "Export saved pairs JSON" + (`/pairs/export`), plain buttons as the concept's; a failed export toasts "The export + could not be downloaded. Nothing was saved to this device." "Import / inspect file" + opens the shared "Import & inspect" (D15, D55), as Images does. +- **D104** (Backup & export, S9, Q4): Recovery policy keeps the concept's "Proposed" chip. + "Create a safety backup before maintenance" is the server's `safety_backup` + (`maintenance.safety_backup`) policy. Its line says what it does ("Keep a verified copy + of the database on disk before maintenance, cleanups and LoRA imports change records. A + restore always keeps one."); the concept's "Preview a verified backup before applying + operations that change records." describes nothing the server does. It is drafted behind the Save row and saved with + `expected_revision` (a configuration changed elsewhere is the conflict status); it is + disabled with "Server settings could not be read." when the read failed. "Scheduled + database backups" offers Manual only; Daily and Weekly stay listed but disabled + (there is no scheduler), the row's line is "Daily and Weekly are not available yet: + back up with Back up whenever you choose." (the concept's proposal line is + prototype copy) and nothing is drafted for it. The warning note reads "Restores are + staged, migrated and validated before replacing live data." +- **D105** (Backup & export, S9): "Review restore…" picks a file (no type filter: the + safety backups `.backup_