From 20dbc2df21ac17a6238b896ae8913e2225ed3ef1 Mon Sep 17 00:00:00 2001 From: Vito Date: Thu, 29 Oct 2026 19:05:21 +0000 Subject: [PATCH] fix(workspace): revalidate served pages and static files on reload (#486) The workspace pages and every file under /prompt_manager/js and /prompt_manager/lib went out with no Cache-Control and no validator. ComfyUI's own cache middleware only fills a missing Cache-Control for paths ending in .js or .css, so the workspace's .mjs modules got no header at all, and after an update the browser could keep running old modules until a hard refresh. Every page and static file now carries Cache-Control: no-cache and a weak ETag over its bytes, and a request that already holds that version gets a 304 without a body. A normal reload picks up new code and stays cheap when nothing changed. Closes #478 --- py/api/__init__.py | 39 ++++++++----- py/api/revalidation.py | 47 +++++++++++++++ tests/test_api_core.py | 126 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 199 insertions(+), 13 deletions(-) create mode 100644 py/api/revalidation.py diff --git a/py/api/__init__.py b/py/api/__init__.py index b2376662..e4b4160c 100644 --- a/py/api/__init__.py +++ b/py/api/__init__.py @@ -40,6 +40,7 @@ from aiohttp import web from PIL import Image from .prompts import PromptRoutesMixin +from .revalidation import revalidated_response from .pairs import PairRoutesMixin from .library import LibraryRoutesMixin from .vocabulary import VocabularyRoutesMixin @@ -653,35 +654,39 @@ class PromptManagerAPI( @routes.get("/prompt_manager/workspace") async def serve_workspace_ui(request: web.Request) -> web.Response: - return self._serve_html_page("workspace.html", "Workspace") + return self._serve_html_page(request, "workspace.html", "Workspace") @routes.get("/prompt_manager/web") async def serve_web_ui(request): - return self._serve_html_page("index.html", "Web UI", cache=False) + return self._serve_html_page(request, "index.html", "Web UI", cache=False) @routes.get("/prompt_manager/gallery.html") async def serve_gallery_ui(request): - return self._serve_html_page("metadata.html", "Gallery") + return self._serve_html_page(request, "metadata.html", "Gallery") @routes.get("/prompt_manager/admin") async def serve_admin_ui(request): - return self._serve_html_page("admin.html", "Admin UI") + return self._serve_html_page(request, "admin.html", "Admin UI") @routes.get("/prompt_manager/gallery") async def serve_gallery_admin_ui(request): - return self._serve_html_page("gallery.html", "Gallery") + return self._serve_html_page(request, "gallery.html", "Gallery") # ── Static file serving ─────────────────────────────────────── @routes.get("/prompt_manager/lib/{filepath:.*}") async def serve_lib_static(request): """Serve static library files (JS, CSS) from web/lib directory.""" - return self._serve_static_file("lib", request.match_info.get("filepath")) + return self._serve_static_file( + request, "lib", request.match_info.get("filepath") + ) @routes.get("/prompt_manager/js/{filepath:.*}") async def serve_js_static(request): """Serve static JavaScript files from web/js directory.""" - return self._serve_static_file("js", request.match_info.get("filepath")) + return self._serve_static_file( + request, "js", request.match_info.get("filepath") + ) # ── Register domain-specific routes from mixins ─────────────── @@ -726,8 +731,11 @@ class PromptManagerAPI( self.logger.info("All routes registered with decorator pattern") - def _serve_static_file(self, subdir, filepath): - """Serve ``web//``; 403 outside it, 404 when missing.""" + def _serve_static_file(self, request, subdir, filepath): + """Serve ``web//``; 403 outside it, 404 when missing. + + A served file is revalidated on every reuse (#478, ``revalidation``). + """ root = os.path.join(_get_project_root(), "web", subdir) exact = STATIC_EXACT_FILES.get((subdir, filepath)) # An exact file passes the same containment check, its extension only. @@ -742,10 +750,13 @@ class PromptManagerAPI( return web.Response(text="Not Found", status=404) with open(file_path, "rb") as f: content = f.read() - return web.Response(body=content, content_type=content_type) + return revalidated_response(request, content, content_type) - def _serve_html_page(self, filename, title, cache=True): - """Serve ``web/``; 404 when missing, 500 when unreadable.""" + def _serve_html_page(self, request, filename, title, cache=True): + """Serve ``web/``; 404 when missing, 500 when unreadable. + + A served page is revalidated on every reuse (#478, ``revalidation``). + """ html_path = os.path.join(_get_project_root(), "web", filename) try: if cache and html_path in self._html_cache: @@ -762,7 +773,9 @@ class PromptManagerAPI( content_type="text/html", status=404, ) - return web.Response(text=content, content_type="text/html", charset="utf-8") + return revalidated_response( + request, content.encode("utf-8"), "text/html", charset="utf-8" + ) except Exception: self.logger.exception(f"Failed to load {title}") return web.Response( diff --git a/py/api/revalidation.py b/py/api/revalidation.py new file mode 100644 index 00000000..6b8f8061 --- /dev/null +++ b/py/api/revalidation.py @@ -0,0 +1,47 @@ +"""Revalidated responses for the pages and static files PromptManager serves. + +``Cache-Control: no-cache`` lets the browser keep a copy but makes it ask the +server before every reuse, so a PromptManager update reaches the browser on a +normal reload (#478). The ETag lets an unchanged file answer 304 without a +body, which keeps that reload cheap. + +The tag hashes the bytes rather than the file's mtime and size: an update can +keep both (an archive extract, a same-length edit within one clock tick), and +the handlers read the whole file anyway. It is weak because the gzip +middleware may send the same content in a different encoding. +""" + +import hashlib + +from aiohttp import web + +REVALIDATE = "no-cache" + + +def entity_tag(body: bytes) -> str: + """Weak ETag for ``body``.""" + return f'W/"{hashlib.sha256(body).hexdigest()[:32]}"' + + +def etag_matches(if_none_match, etag: str) -> bool: + """True when an If-None-Match header names ``etag`` (weak comparison).""" + if not if_none_match: + return False + if if_none_match.strip() == "*": + return True + opaque = etag.removeprefix("W/") + return any( + candidate.strip().removeprefix("W/") == opaque + for candidate in if_none_match.split(",") + ) + + +def revalidated_response(request, body: bytes, content_type: str, charset=None): + """200 with ``body``, or 304 when the request already holds this version.""" + etag = entity_tag(body) + headers = {"Cache-Control": REVALIDATE, "ETag": etag} + if etag_matches(request.headers.get("If-None-Match"), etag): + return web.Response(status=304, headers=headers) + return web.Response( + body=body, content_type=content_type, charset=charset, headers=headers + ) diff --git a/tests/test_api_core.py b/tests/test_api_core.py index bf46c323..7d7d1958 100644 --- a/tests/test_api_core.py +++ b/tests/test_api_core.py @@ -684,6 +684,132 @@ class TestStaticRoutes(CoreRoutesTestCase): self.assertEqual(resp.status, 403) +class TestRevalidatedCaching(CoreRoutesTestCase): + """Every served page and static file is revalidated on reload (#478). + + ``Cache-Control: no-cache`` makes the browser ask before it reuses a copy, + and the ETag lets an unchanged file answer 304 without a body, so an update + reaches the browser on a normal reload and an unchanged reload stays cheap. + """ + + STATIC_PATHS = ( + "/prompt_manager/js/workspace/index.mjs", + "/prompt_manager/js/admin.js", + "/prompt_manager/js/inspection-semantics.json", + "/prompt_manager/lib/viewerjs/viewer.min.css", + ) + + async def _assert_revalidated(self, path): + resp = await self.client.request("GET", path) + self.assertEqual(resp.status, 200, path) + self.assertEqual(resp.headers.get("Cache-Control"), "no-cache", path) + etag = resp.headers.get("ETag") + self.assertTrue(etag, path) + body = await resp.read() + self.assertTrue(body, path) + + again = await self.client.request("GET", path, headers={"If-None-Match": etag}) + self.assertEqual(again.status, 304, path) + self.assertEqual(again.headers.get("ETag"), etag, path) + self.assertEqual(again.headers.get("Cache-Control"), "no-cache", path) + self.assertEqual(await again.read(), b"", path) + + stale = await self.client.request( + "GET", path, headers={"If-None-Match": 'W/"stale"'} + ) + self.assertEqual(stale.status, 200, path) + self.assertEqual(await stale.read(), body, path) + return etag + + async def test_static_files_are_revalidated(self): + for path in self.STATIC_PATHS: + await self._assert_revalidated(path) + + async def test_pages_are_revalidated(self): + for path in UI_PATHS: + await self._assert_revalidated(path) + + async def test_etag_matches_in_a_list_and_as_wildcard(self): + path = "/prompt_manager/js/workspace/index.mjs" + etag = (await self.client.request("GET", path)).headers["ETag"] + for header in (f'W/"other", {etag}', etag.removeprefix("W/"), "*"): + resp = await self.client.request( + "GET", path, headers={"If-None-Match": header} + ) + self.assertEqual(resp.status, 304, header) + + async def test_changed_file_gets_a_new_etag_and_its_new_body(self): + web_dir = self._fake_root() + module = web_dir / "js" / "m.mjs" + module.write_text("export const v = 1;") + path = "/prompt_manager/js/m.mjs" + first = await self.client.request("GET", path) + etag = first.headers["ETag"] + # Same size and, on coarse clocks, the same mtime: only the bytes differ. + module.write_text("export const v = 2;") + resp = await self.client.request("GET", path, headers={"If-None-Match": etag}) + self.assertEqual(resp.status, 200) + self.assertNotEqual(resp.headers["ETag"], etag) + self.assertEqual(await resp.text(), "export const v = 2;") + + async def test_gzipped_and_plain_bodies_share_a_weak_etag(self): + """The gzip middleware changes the bytes, so the tag must be weak.""" + resp = await self.client.request("GET", "/prompt_manager/js/admin.js") + self.assertTrue(resp.headers["ETag"].startswith('W/"')) + + async def test_errors_carry_no_validator(self): + resp = await self.client.request("GET", "/prompt_manager/js/nope.js") + self.assertEqual(resp.status, 404) + self.assertNotIn("ETag", resp.headers) + + +@web.middleware +async def _comfyui_cache_control(request, handler): + """ComfyUI's own middleware (middleware/cache_middleware.py), JS/CSS branch. + + It only fills a missing Cache-Control, and ``.mjs`` is not ``.js``: that is + why the workspace modules went out with no header at all before #478. + """ + response = await handler(request) + if request.path.endswith(".js") or request.path.endswith(".css"): + response.headers.setdefault("Cache-Control", "no-store") + return response + + +class TestRevalidatedCachingBehindMiddlewares(CoreRoutesTestCase): + """The host's middlewares keep the header and the 304 intact.""" + + async def get_application(self): + app = await super().get_application() + app.middlewares.append(_comfyui_cache_control) + app.middlewares.append(api_module._gzip_middleware) + return app + + async def test_gzipped_files_keep_no_cache_and_answer_304(self): + for path in ( + "/prompt_manager/js/workspace/index.mjs", + "/prompt_manager/js/admin.js", + "/prompt_manager/lib/viewerjs/viewer.min.css", + "/prompt_manager/workspace", + ): + resp = await self.client.request( + "GET", path, headers={"Accept-Encoding": "gzip"} + ) + self.assertEqual(resp.status, 200, path) + self.assertEqual(resp.headers.get("Content-Encoding"), "gzip", path) + self.assertEqual(resp.headers["Cache-Control"], "no-cache", path) + again = await self.client.request( + "GET", + path, + headers={ + "Accept-Encoding": "gzip", + "If-None-Match": resp.headers["ETag"], + }, + ) + self.assertEqual(again.status, 304, path) + self.assertEqual(again.headers["Cache-Control"], "no-cache", path) + + # ── instance helpers ──────────────────────────────────────────────────