From 3476f8ede9b62180dff4577d15d8dbe1d132d448 Mon Sep 17 00:00:00 2001 From: Vito Date: Wed, 21 Oct 2026 00:53:04 -0700 Subject: [PATCH] feat(inspector): concept comparison, Show in folder and toast placement (#264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(inspector): concept comparison for the panel and viewer details Compare the Images side panel and the viewer's full details with the concept (1600x1100 and 390x844, both themes): structure, tokens, geometry and controls, plus copy, the acknowledged pair send, the tag chip filter and Prompt history. mapInspectorConcept applies only D3, D20, D21 and D22; a mutation test shows the comparison notices a changed role label and a removed footer. The comparison showed the role copy and send names lacked the concept's ": " subject, so the inspector now passes its title to the prompt blocks. It also showed that a recipe-linked image shows its recipe's own category and tags (#224), which the concept fixture's recipes lack; that is recorded as D38, and the fixture's new positive_links option links the compared image to its positive alone. * docs(inspector): renumber the recipe-category decision to D39 * feat(images): local-only Show in folder route POST /prompt_manager/images/reveal takes the listing identity {root_index, root_scope, path} and opens this computer's file manager at that file. It answers 403 not-local unless the client is loopback, no proxy header is present and Host/Origin name this computer; the scope is checked with _gallery_scope_error (409) and the path resolves only inside that root to a regular media file. Openers run as argv lists with an absolute executable, no shell and DEVNULL streams (D38). * feat(inspector): Show in folder after the File & provenance path The viewer's File & provenance offers "Show in folder" right after the Path value for a listed file only, POSTs its listing identity and toasts the outcome (opened, not-local, unavailable or failed). The browser test server stubs the opener; TESTING.md records the decision as D38. * fix(images): keep the discovery alert clear of the Rescan toast A rescan that leaves the Images discovery alert visible could show its fixed bottom toast over the alert on a short window. The concept's toast keeps its placement; the production-only alert now scrolls into view only as far as needed, with a 96px block-end scroll margin (D42). * docs(inspector): concept panel and viewer details * fix(images): open Windows folders through the shell API, not an Explorer command line Explorer splits its own command line on commas, so a gallery path such as output\a,\Windows\System32\calc.exe,b\x.png escaped the roots through [explorer, "/select,", path] (security review H1). Windows now selects the file with ILCreateFromPathW and SHOpenFolderAndSelectItems, and on any shell failure falls back to os.startfile on the folder. No process is started. * fix(images): re-check, isolate and reap the Show in folder opener - re-check that the real path is still a regular file inside its root right before launching; a changed target answers 404 (review L4) - run openers on a dedicated single worker, not ComfyUI's default executor, and make dbus-send wait for the FileManager1 reply (--print-reply --reply-timeout=2000) so a missing service falls back to xdg-open (L1) - reap launched processes on a daemon thread, give them a minimal environment with a fixed system PATH, and use /usr/bin/open on macOS and dbus-send/xdg-open resolved only on that PATH (L5) * fix(images): accept Show in folder only as a same-origin JSON request, one at a time - require an Origin equal to the request's own scheme://host:port and, when sent, Sec-Fetch-Site: same-origin; refuse every forwarding-style header (Via, X-Forwarded-*, X-Original-*, *-Client-IP, CF-Connecting-IP and the rest) as remote (security review M1, L2) - answer a non-JSON body with 415, so a cross-origin request always needs a CORS preflight (L2) - single flight plus a 1 s cooldown per process: 429 code busy (L1) - path resolution errors such as ENAMETOOLONG are a JSON 404, and neither that nor a start failure logs the client's path at error level (L3) * fix(inspector): say the folder is already opening on a busy Show in folder A 429 busy from the reveal route now toasts "The folder is already opening." The browser workflow checks that the reveal POST carries the page's own Origin and an application/json body, and TESTING.md D38 records the tightened route, the toast table and the accepted residual risk of a header-less local proxy. * test(inspector): compare the viewer details under a coarse pointer and prove its mutations The touch project measured production's D34 40px disclosure targets against the concept's fine-pointer summaries. The concept's plain-chip copy now also carries the recorded D34 coarse rule, so both documents are measured under the same emulation. The open File & provenance is compared too (D38 Show in folder and D20 Download original mapped), and the mutation check now removes the viewer's pair actions and Show in folder. * refactor(inspector): attach Show in folder to the Path value it follows The button was placed on the second fact row by position; the Path dd is now built by the caller and kept, so reordering the facts cannot move it. * test(inspector): let the uncertain-file send check fail again Inspector send buttons are named "... to ComfyUI: <title>", so the anchored regex could never match and the no-send assertion always passed. * fix(images): run the Windows folder fallback inside COM and keep desktop session variables Show in folder's os.startfile fallback now runs inside the same CoInitializeEx/CoUninitialize bracket as the shell selection, as ShellExecute callers should. The Linux opener keeps XAUTHORITY, DESKTOP_SESSION, XDG_SESSION_DESKTOP and XDG_CONFIG_HOME, so a file manager can reach the X11 display under GDM or SDDM and honour the user's mimeapps.list. A Windows-only test calls the real ILCreateFromPathW and ILFree on a temp file and pairs CoInitializeEx with CoUninitialize through the module's helper, without SHOpenFolderAndSelectItems, so no window opens on CI. * fix(images): scroll to the rescan's discovery alert only when the toast band covers it D42 scrolled the alert into view after every rescan, so a user scrolled down the grid was sent back to the top. It now scrolls only when the alert intersects the toast's band (its scroll-margin-block-end). * refactor(inspector): name prompt actions as PR #263 does The title option keeps its name, default and output ("Copy positive prompt: <title>") and now uses #263's actionName helper; the region-name arrow no longer shadows the title option. * test(inspector): the comparison opens Filters by its D44 active-count name After a tag chip applies its filter, the shared toolbar's Filters button reads "Filters · 1" (D44, #263), so the comparison opens it by that name. * fix(images): POSIX opener paths stay absolute under Python 3.13 on Windows Python 3.13 on Windows no longer counts "/usr/bin/x" as absolute, so the faked-Linux reveal tests failed on windows-latest py3.13 (the Linux and macOS openers are POSIX paths whatever host evaluates them). Use posixpath.isabs for those lookups. --- TESTING.md | 99 ++- py/api/images.py | 168 ++++ tests/browser/concept-inventory.mjs | 104 +++ tests/browser/full-inspector.spec.mjs | 46 +- tests/browser/inspector-concept.spec.mjs | 349 ++++++++ tests/browser/prompt-inspector.spec.mjs | 25 +- tests/browser/server.py | 53 ++ tests/browser/workspace-history.spec.mjs | 2 +- tests/browser/workspace-images.spec.mjs | 57 ++ tests/browser/workspace-visual.spec.mjs | 4 +- tests/browser/workspace_concept_fixture.py | 10 +- tests/js/prompt-blocks.test.js | 14 + tests/js/workspace-file-inspector.test.js | 21 + tests/test_image_reveal.py | 919 +++++++++++++++++++++ utils/file_reveal.py | 451 ++++++++++ web/js/workspace/file-inspector.mjs | 65 +- web/js/workspace/images.mjs | 17 +- web/js/workspace/media-viewer.mjs | 4 +- web/js/workspace/prompt-blocks.mjs | 10 +- web/js/workspace/workspace.css | 4 +- 20 files changed, 2381 insertions(+), 41 deletions(-) create mode 100644 tests/browser/inspector-concept.spec.mjs create mode 100644 tests/test_image_reveal.py create mode 100644 utils/file_reveal.py diff --git a/TESTING.md b/TESTING.md index 091f1cb..787bbcc 100644 --- a/TESTING.md +++ b/TESTING.md @@ -225,6 +225,19 @@ managed host with both renderer startup configurations. The shared inline and collection inspector reads the current rooted file's supported embedded records, exact generation facts and recorded or inferred prompt evidence. +It renders in two concept surfaces (#249 D20–D22, D38). The Images side panel shows +only the concept content: cover, title, filename, the evidence badge with a category +chip, the + and − prompt blocks with copy and send, tag chips (the linked positive's +own tags filter Images, D21), the six facts, and a footer with "Send to ComfyUI" and +"Prompt history" only when prompt roles resolved or a link is saved; the sticky panel +scrolls inside the window. The viewer's details (`#viewer-details`) repeat that +content without the cover, then "File & provenance" (the honest file facts, the +`Prompt evidence` line, Download original and, for a listed file, D38's local-only +"Show in folder" after the Path) and "Workflow & raw metadata" (raw records and their +two downloads), then the pair send and Prompt history buttons. D22 words an empty +positive explicitly and makes confirmed prompt text focusable. +`tests/browser/inspector-concept.spec.mjs` compares both surfaces with the concept +(see the D20–D39 notes under Images). `tests/browser/full-inspector.spec.mjs` covers original/metadata/workflow downloads, response limits, source changes, nested sending, matched dark/light and mobile geometry, and the loaded-collection viewer. `viewer matches the concept shell and @@ -592,6 +605,90 @@ Recorded deviations on this page (spec §8): - **D22**: an empty positive reads "No positive prompt (explicitly empty)" (the concept applies its negative wording to both roles), and a confirmed `p.prompt-text` is focusable (`tabindex=0`, marked focus ring) so long prompts stay keyboard-readable. +- **D38 Show in folder** (maintainer decision 2026-10-20, beyond the mock): in the + viewer's File & provenance the Path value stays text, followed in its `dd` by a + "Show in folder" button (`Show <filename> in folder`), only for a listed file + (`root_index`, `root_scope`, `path`), so never in the side panel or a recorded run. + It POSTs that identity as JSON to `/prompt_manager/images/reveal` and toasts: + + | Result | Toast | + |---|---| + | Success | "Opened the folder on this computer." | + | 403 `not-local` | "Show in folder works only in a browser on the computer running ComfyUI." | + | 501 `unavailable` | "No file manager is available on this computer." | + | 429 `busy` | "The folder is already opening." | + | Anything else (415, 404, 409, 500, offline) | "The folder could not be opened. Try again." | + + The route is local-only. It answers 403 `not-local` unless all of these hold: no + forwarding-style header (`Forwarded`, `Via`, `X-Real-IP`, `CF-Connecting-IP`, + `True-Client-IP`, `X-Client-IP`, `X-Cluster-Client-IP`, `Fastly-Client-IP`, and any + `X-Forwarded-*`, `X-Original-*` or `*-Client-IP`); a loopback `request.remote`; a + loopback `Host` (`localhost`, `*.localhost` or a loopback address, against DNS + rebinding); an `Origin` equal to the request's own scheme, host and port (absent, + `null`, another port or another scheme is refused; browsers send `Origin` on every + fetch POST); and, when sent, `Sec-Fetch-Site: same-origin`. A body that is not + `application/json` is 415 `unsupported-media-type`, so any cross-origin request + needs a CORS preflight. The body is strict (an int root, a 64-hex scope, a bounded + relative path; absolute, drive and `..` paths are 400), the scope goes through + `_gallery_scope_error` (409 `source-changed`), and the path resolves only inside + that root to a regular media file (403/404; a resolution `OSError` such as + `ENAMETOOLONG` is the same JSON 404, and the client's path is never logged at error + level). One reveal runs at a time per process with a 1 s cooldown after it + (`RevealGate`, 429 `busy`), on its own single worker (`reveal_executor`), never + ComfyUI's default executor. Just before launching, `utils/file_reveal.py` re-checks + that the real path is still a regular file inside the root (a swapped parent is + 404). Windows starts no process: `ILCreateFromPathW` and + `SHOpenFolderAndSelectItems` select the file through the shell API (Explorer's own + command line splits on commas, so a path like `output\a,\Windows\...` must never + reach it), falling back to `os.startfile(folder, "explore")`. macOS runs + `/usr/bin/open -R`; Linux asks FileManager1 `ShowItems` with `dbus-send + --print-reply --reply-timeout=2000` (a real reply or a fallback) and else runs + `xdg-open` on the folder. Those openers are argv lists resolved only on a fixed + system PATH, with no shell, DEVNULL streams, `close_fds`, their own session, a + minimal environment (no venv PATH, PYTHONPATH or API keys) and a daemon-thread + reaper. `tests/test_image_reveal.py` mocks `Popen`, `sys.platform`, `shutil.which`, + the shell API and `os.startfile`; the browser server's opener is a stub + (`POST`/`GET /__test__/reveal`) with no cooldown. The inspector comparison reads + closed disclosures as hidden, so the button needs no mapping there. + + Residual risk, accepted: a reverse proxy or tunnel that adds no forwarding header + (a bare nginx `proxy_pass` to 127.0.0.1, `ssh -R`, `socat`) cannot be told apart + from a local client, and a non-browser client behind it can forge a matching + `Host` and `Origin`. Such a client can open a folder of a listed file on the host's + screen, at most once a second, and learn whether that file exists (which the + listing routes already tell it). Operators who expose ComfyUI that way accept this. +- **D39**: a recipe-linked image shows its recipe's own category and tags (#224), not + its positive's. The concept shows the prompt's category and tags for every image; + the concept fixture's recipes have neither, so their images show no category chip + and no tag row. `positive_links` on `/__test__/concept/reset` links the newest + images to their positive alone, so the inspector comparison sees the positive's + category ("Positive prompt category: " hidden prefix, D21) and tag filters. +- **D42**: the concept never shows a toast and a page alert together. Production keeps + the concept's fixed toast (centred, 22px from the bottom, 12px at 700px and below) + and, when an Import → Rescan toast leaves the Images discovery alert + (`.options-status`), scrolls that alert into view only as far as needed, with + `scroll-margin-block-end: 96px`, so it is never under the toast. + `Rescan toast does not cover the discovery alert it leaves` ends the window inside + the toast's band at 1600 and 390 pixels and measures both boxes. + +`tests/browser/inspector-concept.spec.mjs` compares the side panel and the viewer's +full details (#249 D4) for alpine_00001.png, at 1600x1100 and 390x844 in both themes: +structure, cover, heading, filename, badges, prompt blocks (role labels, actions, +text), tag chips, facts, footer, the viewer's disclosures and pair actions, and every +offered control's name and minimum height. Each role's copy and send are named after +the prompt, as the concept's are ("Copy positive prompt: <title>"). The concept loads +all 24 fixture images first (D37). Behaviors: copy shows "Prompt copied." (or the +same "Copy prompt" dialog in both); the pair send previews both roles in the concept +and is applied and acknowledged by a test ComfyUI tab in production; the `cinematic` +chip sets the Images Tags filter (D21: the concept searches the word, which also +matches the untagged forest prompt's text); Prompt history goes to +`#/library/<positive id>` (the concept's `#prompt/<id>`). `mapInspectorConcept` +applies D3 (the `.file-inspector` class of the production-only rules), D20 (the +viewer's plain tag chips, laid out by the concept page itself with its chips made +plain at runtime), the D20/D21 side-panel scroll (`overflow: auto`, +`max-height: calc(100dvh - 40px)` above 700px), D21 (the category's hidden scope +prefix) and D22 (focusable prompt text). `inspector comparison detects mutations` +changes `.prompt-heading .role-label` letter spacing and removes `.detail-footer`. Manual check (quick), after a browser refresh on Images: hover an image (it zooms); click it (the viewer reads "<title> · <filename>"; Escape closes it); press a card's @@ -601,7 +698,7 @@ saved file. ```sh npm run --prefix tools/vendor test:workflows -- images-concept.spec.mjs \ - workspace-images.spec.mjs full-inspector.spec.mjs \ + workspace-images.spec.mjs full-inspector.spec.mjs inspector-concept.spec.mjs \ --project chromium --project touch --project firefox --project webkit ``` diff --git a/py/api/images.py b/py/api/images.py index 75a51d1..aef3656 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -38,6 +38,14 @@ try: from ...utils.parallel import map_parallel from ...utils.inspection_response import encode_inspection_response from ...utils.prompt_inspector import MetadataReadError, read_inspection_file + from ...utils.file_reveal import ( + OpenerUnavailable, + RevealGate, + RevealTargetChanged, + is_local_request, + reveal_executor, + reveal_in_file_manager, + ) except ImportError: from database.image_favorites import MAX_KEY_LENGTH from database.library import display_title @@ -46,6 +54,14 @@ except ImportError: from utils.parallel import map_parallel from utils.inspection_response import encode_inspection_response from utils.prompt_inspector import MetadataReadError, read_inspection_file + from utils.file_reveal import ( + OpenerUnavailable, + RevealGate, + RevealTargetChanged, + is_local_request, + reveal_executor, + reveal_in_file_manager, + ) IMAGE_EXTENSIONS = _IMAGE_EXTENSIONS @@ -522,6 +538,51 @@ def _favorite_target(data: Mapping, roots: list[Path]): return (index, relative), None +def _reveal_target(data: Mapping): + """``((index, scope, relative), None)`` for a reveal body, else an error. + + Only the listing identity is accepted: an int root index, the root's + 64-hex scope and a bounded root-relative path, never an absolute, + drive-qualified or ``..`` path. + """ + index, scope, relative = ( + data.get("root_index"), + data.get("root_scope"), + data.get("path"), + ) + if type(index) is not int or index < 0: + return None, bad_request("Invalid gallery root") + if not isinstance(scope, str) or re.fullmatch(r"[0-9a-f]{64}", scope) is None: + return None, bad_request("Invalid gallery root scope") + if ( + not isinstance(relative, str) + or not relative + or len(relative) > MAX_FAVORITE_PATH + or "\0" in relative + or _has_traversal(relative) + or re.match(r"^[a-zA-Z]:", relative) + ): + return None, bad_request("path must be a relative media path") + return (index, scope, relative), None + + +def _reveal_not_found(): + return web.json_response( + {"success": False, "error": "Image file not found"}, status=404 + ) + + +def _not_local(): + return web.json_response( + { + "success": False, + "code": "not-local", + "error": "Show in folder works only on the computer running ComfyUI.", + }, + status=403, + ) + + # The path module whose case rules key favorites (tests swap in ntpath). _path_flavor = os.path @@ -828,6 +889,11 @@ class ImageRoutesMixin: async def clear_image_favorite_route(request): return await self.update_image_favorite(request, favorite=False) + # POST, local-only: opens this computer's file manager (D38). + @routes.post("/prompt_manager/images/reveal") + async def reveal_image_route(request): + return await self.reveal_image(request) + # POST: drops the server-side listing cache and rewalks the output roots. @routes.post("/prompt_manager/gallery/rescan") async def rescan_gallery_route(request): @@ -2156,6 +2222,108 @@ class ImageRoutesMixin: } ) + # Swapped by tests and the disposable browser server; never a shell. + _reveal_opener = staticmethod(reveal_in_file_manager) + # One per process (a class attribute): single flight plus a cooldown. + _reveal_gate = RevealGate() + + @staticmethod + def _reveal_path(relative: str, root: Path) -> Path | None: + """The real path of a regular media file under *root*, else None.""" + resolved = ImageRoutesMixin._resolve_client_media_path(relative, [root]) + if resolved is None or _media_access_error(resolved, [root]) is not None: + return None + real = Path(os.path.realpath(resolved)) + return real if real.is_file() and _is_within(real, root) else None + + async def reveal_image(self, request): + """POST ``{root_index, root_scope, path}``: show the file in its folder. + + Local-only (a same-origin browser request from a loopback client, no + forwarding header) and JSON-only: the browser must run on the + computer whose file manager opens. One reveal at a time (429 busy). + """ + if not is_local_request(request.remote, request.headers, request.scheme): + return _not_local() + if request.content_type != "application/json": + # A non-JSON POST is a CORS "simple" request with no preflight. + return web.json_response( + { + "success": False, + "code": "unsupported-media-type", + "error": "Send the request as application/json.", + }, + status=415, + ) + data, error = await _json_body_or_error(request) + if error is not None: + return error + target, error = _reveal_target(data) + if error is not None: + return error + index, scope, relative = target + roots = list(self._get_all_output_dirs()) + error = self._gallery_scope_error( + {"root_scope": scope, "root": str(index)}, roots + ) + if error is not None: + return error + root = roots[index] + if Path(relative).suffix.lower() not in SERVABLE_EXTENSIONS: + return _forbidden("Only media files can be shown") + gate = self._reveal_gate + if not gate.try_enter(): + return web.json_response( + { + "success": False, + "code": "busy", + "error": "The folder is already opening.", + }, + status=429, + ) + try: + return await self._reveal(relative, root) + finally: + gate.leave() + + async def _reveal(self, relative: str, root: Path): + """Resolve and open, inside the reveal gate.""" + try: + path = await self._run_in_executor(self._reveal_path, relative, root) + except (OSError, ValueError) as exc: + # ENAMETOOLONG and the like: the client's path is not logged. + self.logger.debug( + "Show in folder could not resolve: %s", type(exc).__name__ + ) + return _reveal_not_found() + if path is None: + return _reveal_not_found() + try: + await asyncio.get_running_loop().run_in_executor( + reveal_executor(), self._reveal_opener, path, root + ) + except RevealTargetChanged: + return _reveal_not_found() + except OpenerUnavailable: + return web.json_response( + { + "success": False, + "code": "unavailable", + "error": "No file manager is available on this computer.", + }, + status=501, + ) + except Exception as exc: + # The exception text can hold the path: only its type at error + # level, the traceback at debug. + self.logger.error("Show in folder failed: %s", type(exc).__name__) + self.logger.debug("Show in folder failure", exc_info=True) + return web.json_response( + {"success": False, "error": "The folder could not be opened."}, + status=500, + ) + return web.json_response({"success": True}) + def _favorite_listing(self, root: Path, files: list) -> dict[str, str]: """``{relative key: listing POSIX spelling}`` for one root's scan. diff --git a/tests/browser/concept-inventory.mjs b/tests/browser/concept-inventory.mjs index 2ec0aee..35efb58 100644 --- a/tests/browser/concept-inventory.mjs +++ b/tests/browser/concept-inventory.mjs @@ -452,3 +452,107 @@ export function measureInventory(scope) { panel:when(gallery,panelOf), library:libraryOf(),history:null,noPageScroll:document.documentElement.scrollWidth<=innerWidth+1}; } + +/** + * The inspector (#249 D4): `panel` is the Images side panel's content (`aside#image-inspector`, app.js imageDetails + * a400–414) and `viewerDetails` the viewer's full details (`aside#viewer-details`, imageDetails(i, true)), each null + * when absent. Both are measured by one in-page function with the concept's selectors in either document. + */ +export async function inspectorOf(page) { + return {panel:await page.evaluate(measureInspector,'#image-inspector'), + viewerDetails:await page.evaluate(measureInspector,'#viewer-details')}; +} + +/** Runs in the page; self-contained. Every box is relative to the measured root. */ +export function measureInspector(selector) { + const root=document.querySelector(selector); + if(!root)return null; + const px=value=>Math.round(value); + const css=(node,...keys)=>{const style=getComputedStyle(node);return Object.fromEntries(keys.map(key=>[key,style[key]]));}; + const shown=node=>Boolean(node)&&node.getClientRects().length>0&&getComputedStyle(node).visibility!=='hidden'; + const rect=node=>node.getBoundingClientRect(); + const font=node=>css(node,'fontFamily','fontSize','fontWeight','lineHeight','letterSpacing','color','textTransform'); + const edges=node=>css(node,'paddingTop','paddingRight','paddingBottom','paddingLeft','marginTop','marginRight','marginBottom','marginLeft', + 'borderTopWidth','borderTopStyle','borderTopColor','borderRightWidth','borderBottomWidth','borderTopLeftRadius','backgroundColor'); + const layout=node=>css(node,'display','flexDirection','flexWrap','justifyContent','alignItems','columnGap','rowGap'); + const box=node=>{const a=rect(node),b=rect(root);return {left:px(a.left-b.left),top:px(a.top-b.top),width:px(a.width),height:px(a.height)};}; + const classes=node=>[...node.classList].sort().join(' '); + const when=(node,measure)=>node?measure(node):null; + // Visible text: without visually hidden prefixes and aria-hidden glyphs, whitespace collapsed. + const textOf=(node,skip='.ca-visually-hidden, [aria-hidden="true"]')=>{const copy=node.cloneNode(true);copy.querySelectorAll(skip).forEach(item=>item.remove()); + return copy.textContent.replace(/\s+/g,' ').trim();}; + const hiddenOf=node=>[...node.querySelectorAll('.ca-visually-hidden')].map(item=>item.textContent).join('')||null; + const nameOf=node=>node.getAttribute('aria-label')??textOf(node,'[aria-hidden="true"]'); + function iconOf(node) { + const svg=node.querySelector(':scope > svg.icon'); + if(!svg)return null; + return {width:px(rect(svg).width),height:px(rect(svg).height),viewBox:svg.getAttribute('viewBox'),hidden:svg.getAttribute('aria-hidden'), + d:svg.querySelector('path')?.getAttribute('d')??null,...css(svg,'strokeWidth','color')}; + } + function controlOf(node) { + const style=getComputedStyle(node); + return {tag:node.tagName.toLowerCase(),className:classes(node),text:textOf(node),name:nameOf(node),pressed:node.getAttribute('aria-pressed'), + primary:node.classList.contains('ca-primary'),...box(node),...font(node),...edges(node),...layout(node), + minHeight:style.minHeight,minWidth:style.minWidth,icon:iconOf(node)}; + } + function chipOf(node) { + return {tag:node.tagName.toLowerCase(),className:classes(node),text:textOf(node),hidden:hiddenOf(node),...box(node),...font(node),...edges(node), + minHeight:getComputedStyle(node).minHeight}; + } + function rowOf(row) { + return {...layout(row),...edges(row),...box(row),chips:[...row.children].map(chipOf)}; + } + function sectionOf(section) { + const heading=section.querySelector(':scope > .prompt-heading'),label=heading?.querySelector('.role-label'); + const actions=heading?.querySelector('.prompt-actions'),text=section.querySelector(':scope > .prompt-text'); + return {className:classes(section),...edges(section),...box(section), + heading:when(heading,()=>({...layout(heading),...edges(heading),...box(heading)})), + label:when(label,()=>({tag:label.tagName.toLowerCase(),text:label.textContent,...font(label),...box(label)})), + actions:when(actions,()=>({...layout(actions),...box(actions),buttons:[...actions.children].map(controlOf)})), + text:when(text,()=>({tag:text.tagName.toLowerCase(),className:classes(text),text:text.textContent,tabIndex:text.tabIndex,...font(text),...edges(text), + ...css(text,'whiteSpace','overflowWrap'),...box(text),lines:px(rect(text).height/parseFloat(getComputedStyle(text).lineHeight))}))}; + } + function factsOf(list) { + const style=getComputedStyle(list); + return {...css(list,'display','columnGap','rowGap','fontSize'),...edges(list),columns:style.gridTemplateColumns.split(/\s+/).length,...box(list), + items:[...list.children].map(item=>{const dt=item.querySelector('dt'),dd=item.querySelector('dd'); + return {...box(item),dt:when(dt,()=>({text:dt.textContent,...font(dt),...edges(dt)})),dd:when(dd,()=>({text:dd.textContent,...font(dd),...edges(dd)}))};})}; + } + const describe=item=>`${item.localName}${item.classList.length?'.'+[...item.classList].join('.'):''}`; + const panel=root.classList.contains('detail-panel'); + // The panel's `.detail-inner`, the viewer's class-less wrapper (D20). A prompt-block mount point that is + // `display: contents` is transparent: its sections count as the wrapper's own children. + const inner=root.querySelector(panel?':scope > .detail-inner':':scope > div'); + const parts=inner?[...inner.children].flatMap(item=>getComputedStyle(item).display==='contents'?[...item.children]:[item]):[]; + const sections=parts.filter(item=>item.classList.contains('prompt-section')); + const firstSection=parts.indexOf(sections[0]); + const rows=parts.filter(item=>item.classList.contains('tags')); + const heading=parts.find(item=>item.classList.contains('panel-heading')),h2=heading?.querySelector('h2'),format=heading?.querySelector('.chip'); + const filename=parts.find(item=>item.matches('p.small')); + const cover=root.querySelector(':scope > .detail-cover'),footer=root.querySelector(':scope > .detail-footer'); + // What a closed disclosure holds is not offered (only its summary is), whatever boxes the engine keeps for it. + const offered=node=>{const closed=node.closest('details:not([open])');return !closed||node.parentElement===closed&&node.localName==='summary';}; + const controls=[...root.querySelectorAll('button, a[href], [tabindex]:not([tabindex="-1"]), summary')].filter(node=>shown(node)&&offered(node)); + return { + className:classes(root),label:root.getAttribute('aria-label'), + ...css(root,'position','top','overflowX','overflowY','maxHeight'),...edges(root),width:px(rect(root).width),height:px(rect(root).height), + structure:{outer:[...root.children].map(describe),inner:parts.map(describe)}, + cover:when(cover,()=>({tag:cover.tagName.toLowerCase(),alt:cover.getAttribute('alt'),...box(cover),...css(cover,'objectFit','display')})), + inner:when(inner,()=>({...edges(inner),...box(inner)})), + heading:when(heading,()=>({...layout(heading),...edges(heading),...box(heading), + h2:when(h2,()=>({text:h2.textContent,...font(h2),...edges(h2),...box(h2)})),format:when(format,chipOf)})), + filename:when(filename,()=>({text:filename.textContent,...font(filename),...edges(filename),...box(filename)})), + badges:when(rows.find(row=>parts.indexOf(row)<firstSection),rowOf), + sections:sections.map(sectionOf), + tags:when(rows.find(row=>parts.indexOf(row)>firstSection),rowOf), + facts:when(parts.find(item=>item.matches('dl.meta-grid')),factsOf), + // Disclosures (viewer only) by their summary and frame; their content is the D20 production facts. + details:parts.filter(item=>item.localName==='details').map(item=>{const summary=item.querySelector(':scope > summary'); + return {open:item.open,...edges(item),...box(item),summary:when(summary,()=>({text:summary.textContent,...font(summary),...box(summary)}))};}), + footer:when(footer,()=>({...layout(footer),...edges(footer),...box(footer),buttons:[...footer.children].map(controlOf)})), + // The viewer's pair send and history follow the wrapper directly (imageDetails(i, true) has no footer). + actions:panel?null:[...root.children].filter(item=>item.localName==='button').map(controlOf), + controls:controls.map(node=>({tag:node.tagName.toLowerCase(),text:textOf(node),name:nameOf(node),pressed:node.getAttribute('aria-pressed'), + primary:node.classList.contains('ca-primary'),minHeight:getComputedStyle(node).minHeight})), + }; +} diff --git a/tests/browser/full-inspector.spec.mjs b/tests/browser/full-inspector.spec.mjs index 528a2cd..3630d45 100644 --- a/tests/browser/full-inspector.spec.mjs +++ b/tests/browser/full-inspector.spec.mjs @@ -243,7 +243,8 @@ test('viewer details hold provenance, raw records and downloads',async({page,req const provenance=details.locator('details').first();await provenance.locator('summary').click(); await expect(provenance.locator('p.small').first()).toHaveText('Current file at this source; historical bytes are not verified.'); await expect(provenance.locator('dl.meta-grid dt')).toHaveText(['Source','Path','Size','Dimensions','Format','Media type','Modified']); - await expect(provenance.locator('dl.meta-grid dd').nth(1)).toHaveText('inspector/complete.png'); + // D38: the Path value stays text, with "Show in folder" right after it. + await expect(provenance.locator('dl.meta-grid dd').nth(1)).toHaveText('inspector/complete.png Show in folder'); await expect(provenance.getByText('Prompt evidence: workflow inferred',{exact:true})).toBeVisible(); await expect(provenance.getByText('Saved association; not verified against current file',{exact:true})).toBeVisible(); await expect(provenance.getByRole('link',{name:'Download original',exact:true})).toBeVisible(); @@ -271,6 +272,41 @@ test('viewer details hold provenance, raw records and downloads',async({page,req await expect(page.locator('#viewer-dialog')).toHaveCount(0); }); +// D38 Show in folder: a listed file's Path offers it; the stubbed opener records what the route resolved. +test('Show in folder asks the server to open the listed file and announces each outcome',async({page,request})=>{ + await fullFixture(page,request); + await expect(inspector(page).getByRole('button',{name:/in folder$/})).toHaveCount(0); + const details=await viewerDetails(page,'complete.png'); + const provenance=details.locator('details').first();await provenance.locator('summary').click(); + const path=provenance.locator('dl.meta-grid > div').nth(1);await expect(path.locator('dt')).toHaveText('Path'); + const show=path.locator('dd > button'); + await expect(show).toHaveText('Show in folder');await expect(show).toHaveAccessibleName('Show complete.png in folder'); + const toast=page.locator('#workspace-toast'); + const [sent]=await Promise.all([page.waitForRequest(entry=>entry.url().endsWith('/prompt_manager/images/reveal')),show.click()]); + expect(sent.method()).toBe('POST'); + // The route takes only a same-origin JSON request, as a real browser sends it. + const sentHeaders=await sent.allHeaders(); + expect(sentHeaders.origin).toBe(new URL(page.url()).origin);expect(sentHeaders['content-type']).toBe('application/json'); + const body=sent.postDataJSON(); + expect(Object.keys(body).sort()).toEqual(['path','root_index','root_scope']); + expect(body).toMatchObject({root_index:0,path:'inspector/complete.png'});expect(body.root_scope).toMatch(/^[0-9a-f]{64}$/); + await expect(toast).toHaveText('Opened the folder on this computer.'); + const opened=async()=>(await (await request.get('/__test__/reveal')).json()).opened; + expect(await opened()).toEqual(['output/inspector/complete.png']); + for (const [outcome,message] of [['unavailable','No file manager is available on this computer.'],['fail','The folder could not be opened. Try again.']]) { + await expect(await request.post('/__test__/reveal',{data:{outcome}})).toBeOK(); + await show.click();await expect(toast).toHaveText(message); + } + // A reveal already in progress answers 429 busy. + await page.route('**/prompt_manager/images/reveal',route=>route.fulfill({status:429,contentType:'application/json',body:JSON.stringify({success:false,code:'busy',error:'The folder is already opening.'})})); + await show.click();await expect(toast).toHaveText('The folder is already opening.'); + await page.unroute('**/prompt_manager/images/reveal'); + // A proxied request counts as remote, so the real route answers 403 not-local. + await expect(await request.post('/__test__/reveal',{data:{outcome:'opened'}})).toBeOK(); + await page.route('**/prompt_manager/images/reveal',route=>route.continue({headers:{...route.request().headers(),'x-forwarded-for':'203.0.113.7'}})); + await show.click();await expect(toast).toHaveText('Show in folder works only in a browser on the computer running ComfyUI.'); + expect(await opened()).toEqual(['output/inspector/complete.png']); +}); test('full inspector shows six facts and scoped provenance',async({page,request},testInfo)=>{ await page.setViewportSize({width:1600,height:1100});await fullFixture(page,request);const panel=inspector(page); await expect(panel.getByRole('heading',{name:POSITIVE_TITLE,exact:true})).toBeVisible();await expect(panel.locator('.panel-heading .chip')).toHaveText('PNG');await expect(panel.locator('.detail-inner > p.small')).toHaveText('complete.png'); @@ -278,12 +314,12 @@ test('full inspector shows six facts and scoped provenance',async({page,request} expect(await panel.locator('.detail-inner').evaluate(node=>getComputedStyle(node).padding)).toBe('17px'); if (!await page.evaluate(()=>matchMedia('(pointer: coarse)').matches)) { // The concept's prompt actions (`.prompt-actions button.icon-btn`) measure 31×31 in the mock. - const copy=await panel.getByRole('button',{name:'Copy positive prompt',exact:true}).boundingBox();expect(copy.width).toBe(31);expect(copy.height).toBe(31); + const copy=await panel.getByRole('button',{name:/^Copy positive prompt: /}).boundingBox();expect(copy.width).toBe(31);expect(copy.height).toBe(31); } await expect(panel.locator('.meta-grid dt')).toHaveText(['Seed','Steps','CFG','Sampler','Scheduler','Model']); await expect(panel.locator('.meta-grid dd')).toHaveText(['9007199254740993','28','6.5','dpmpp_2m','karras','fixture-model.safetensors']); await selectFile(page,'empty.png');await expect(panel.locator('.panel-heading .chip')).toHaveText('PNG');await expect(panel.getByRole('heading',{name:'empty.png',exact:true})).toBeVisible(); - await expect(panel.getByRole('button',{name:'Send negative prompt to ComfyUI',exact:true})).toHaveCount(0);await expect(panel).toContainText('Not recorded in this file.');await expect(panel.getByRole('button',{name:'Prompt history',exact:true})).toHaveCount(0); + await expect(panel.getByRole('button',{name:/^Send negative prompt to ComfyUI: /})).toHaveCount(0);await expect(panel).toContainText('Not recorded in this file.');await expect(panel.getByRole('button',{name:'Prompt history',exact:true})).toHaveCount(0); await selectFile(page,'explicit-empty.png');await expect(panel.locator('.prompt-section.negative p.prompt-text')).toHaveText('No negative prompt (explicitly empty)');await expect(panel.getByRole('button',{name:/^Send prompt pair/})).toHaveCount(1); // Provenance lives in the viewer's full details (D20): one "Prompt evidence" line replaces the per-fact source lines. const details=await viewerDetails(page,'complete.png'); @@ -356,7 +392,7 @@ test('full inspector keeps long literal content reachable on mobile',async({page const literal='<svg onload="window.executed=true"><image href="https://invalid.test/never"/></svg></pre>'+'verylongmodelpath/'.repeat(30); let external=0;await page.route('https://invalid.test/**',route=>{external++;return route.abort();}); await page.route('**/images/inspect/**',async route=>{const response=await route.fetch();const data=await response.json();data.metadata.raw.parameters=literal;data.metadata.record_formats.parameters='original-text';data.metadata.parameters=literal;data.metadata.prompt_graph.value['4'].inputs.ckpt_name=literal;await route.fulfill({response,json:data});}); - await page.setViewportSize({width:390,height:844});await fullFixture(page,request);const panel=inspector(page);await expect(panel.locator('.prompt-section.positive p.prompt-text')).toContainText('fine geometry and light.');await panel.getByRole('button',{name:'Copy positive prompt',exact:true}).click();const fallback=page.getByRole('dialog',{name:'Copy prompt',exact:true});await expect(fallback.getByRole('textbox')).toHaveValue(/^Inspector portrait 猫 😀\nFull prompt context, fine geometry and light\./);await page.keyboard.press('Escape');await expect(fallback).toBeHidden(); + await page.setViewportSize({width:390,height:844});await fullFixture(page,request);const panel=inspector(page);await expect(panel.locator('.prompt-section.positive p.prompt-text')).toContainText('fine geometry and light.');await panel.getByRole('button',{name:/^Copy positive prompt: /}).click();const fallback=page.getByRole('dialog',{name:'Copy prompt',exact:true});await expect(fallback.getByRole('textbox')).toHaveValue(/^Inspector portrait 猫 😀\nFull prompt context, fine geometry and light\./);await page.keyboard.press('Escape');await expect(fallback).toBeHidden(); await expect(panel.locator('.meta-grid dd').last()).toHaveText(literal); for(const zoom of [1,2]) {await page.evaluate(zoom=>{document.documentElement.style.zoom=String(zoom);},zoom);await expect.poll(()=>page.evaluate(()=>document.documentElement.scrollWidth<=innerWidth+1)).toBe(true);const footer=panel.locator('.detail-footer button.send');await footer.focus();await expect(footer).toBeFocused();} await page.evaluate(()=>{document.documentElement.style.zoom='1';}); @@ -719,7 +755,7 @@ test('slideshow interaction visibility sender and disposal clear intent',async({ for(const action of [ ()=>modal.locator('.prompt-section.positive p.prompt-text').click(), ()=>modal.locator('#viewer-details').dispatchEvent('wheel',{deltaY:100}), - ()=>modal.getByRole('button',{name:'Copy positive prompt',exact:true}).click(), + ()=>modal.getByRole('button',{name:/^Copy positive prompt: /}).click(), async()=>{await modal.getByText('Workflow & raw metadata',{exact:true}).click();await downloadText(page,modal.getByRole('button',{name:'Download metadata JSON',exact:true}));}, async()=>{await page.evaluate(()=>{Object.defineProperty(document,'hidden',{configurable:true,value:true});document.dispatchEvent(new Event('visibilitychange'));Object.defineProperty(document,'hidden',{configurable:true,value:false});document.dispatchEvent(new Event('visibilitychange'));});}, async()=>{const trigger=modal.getByRole('button',{name:/^Send prompt pair to ComfyUI: /});await trigger.click();await expect(page.getByRole('dialog',{name:'Send to ComfyUI',exact:true})).toBeVisible();await page.keyboard.press('Escape');await expect(trigger).toBeFocused();}, diff --git a/tests/browser/inspector-concept.spec.mjs b/tests/browser/inspector-concept.spec.mjs new file mode 100644 index 0000000..6090796 --- /dev/null +++ b/tests/browser/inspector-concept.spec.mjs @@ -0,0 +1,349 @@ +import {writeFile} from 'node:fs/promises'; +import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {captureCanonicalState} from './history-canonical-comparison.mjs'; +import {SIZES,THEMES,seedConcept,openConcept,openProduction,inspectorOf,diffInventory,mutate,countCopy} from './concept-inventory.mjs'; + +// Task D4 of #249: the Images side panel (app.js imageDetails a400–414) and the viewer's full details (imageDetails(i, +// true)) against the immutable concept, on the concept-shaped real fixture, at 1600x1100 and 390x844 in both themes. +// Both documents select the first listed image, alpine_00001.png (D19: the first load selects it), which the fixture +// links to its positive prompt alone (`positive_links`, D39), so its panel shows the positive's own category and tags +// as the concept's does. The panel's box in the gallery layout is the Images spec's. Paired captures are artifacts only; +// there are no PNG baselines. +test.use({locale:'en-US',timezoneId:'UTC'}); + +const CARDS='.gallery-body > .image-grid > .media-card'; +const TITLE='Alpine observatory'; +const FIXTURE_IMAGES=24; +const SEED={positive_links:1}; +// The concept's sticky panel never scrolls; production's scrolls inside the window above 700px (D20/D21 side panel). +const PANEL_INSET=40; +// D34: coarse pointers keep 40px targets for the viewer details' disclosure summaries and "Download original" (the +// concept's coarse rule covers buttons only). The recorded rule, applied to the concept under the same emulation so the +// browser lays out what follows the taller summaries; a fine pointer never matches it. +const D34_COARSE_TARGETS=`@media (pointer: coarse) { + #viewer-details summary { box-sizing: border-box; min-block-size: 40px; padding-block: 10px; } + #viewer-details a { display: inline-flex; align-items: center; min-block-size: 40px; } +}`; +// What an open "File & provenance" offers beyond the concept's text-only disclosure: D38's Show in folder right after +// the Path value, then D20's Download original. +const FILE_CONTROLS=[{tag:'button',text:'Show in folder',name:'Show alpine_00001.png in folder'},{tag:'a',text:'Download original',name:'Download original'}]; + +/** D22: production's confirmed prompt text is focusable, so it follows its block's actions among the controls. */ +function withFocusablePrompts(inventory) { + const controls=[...inventory.controls]; + for(const section of inventory.sections) { + const send=section.actions.buttons.at(-1); + const index=controls.findIndex(control=>control.name===send.name); + controls.splice(index+1,0,{tag:'p',text:section.text.text,name:section.text.text,pressed:null,primary:false,minHeight:'0px'}); + } + return {...inventory,controls,sections:inventory.sections.map(section=>({...section,text:{...section.text,tabIndex:0}}))}; +} + +/** D21: the category chip names its scope in a visually hidden prefix; here the linked positive's. */ +function withCategoryScope(inventory) { + const [badge,category,...rest]=inventory.badges.chips; + return {...inventory,badges:{...inventory.badges,chips:[badge,{...category,hidden:'Positive prompt category: '},...rest]}}; +} + +/** + * Recorded inspector deviations only (spec §8, TESTING.md D3, D20–D22, D34, D38 and D39 through the fixture's + * `positive_links`); everything else must equal production. `reference.viewerPlain` is the concept's own full details + * with its tag chips made plain (D20) and, under a coarse pointer, D34's targets, both at runtime: the browser lays out + * what follows them, as the Images spec's removed toolbar items do. + */ +export function mapInspectorConcept(reference,{viewport}) { + const {panel,viewerPlain,fileControls}=reference; + return { + panel:withCategoryScope(withFocusablePrompts({...panel, + // D3: the production-only inspector rules (`.file-inspector`, workspace.css) hook on this class. + className:`${panel.className} file-inspector`, + // D20/D21 side panel: the sticky panel scrolls inside the window, so long real prompts keep the footer + // reachable; at 700px and below, where the concept unsticks it, it grows instead. + overflowX:'auto',overflowY:'auto',maxHeight:viewport.width>700?`${viewport.height-PANEL_INSET}px`:'none'})), + // D3 as above; D20: the viewer's tag chips are plain (the concept's go to the Library). + viewerDetails:withCategoryScope(withFocusablePrompts({...viewerPlain,className:'file-inspector'})), + // D38 and D20: the open "File & provenance" offers Show in folder and Download original. + fileControls:[...fileControls,...FILE_CONTROLS], + }; +} + +function compare(reference,actual,viewport) { + return diffInventory(mapInspectorConcept(reference,{viewport}),actual); +} + +/** The comparison as a gate: throws, naming every differing path, unless production equals the mapped concept. */ +function assertInspectorMatch(reference,actual,viewport) { + const diff=compare(reference,actual,viewport); + if(diff.length)throw new Error(`the inspector differs from the concept at ${diff.map(item=>item.path).join(', ')}`); +} + +// Two frames, then every running transition finished. +const settle=page=>page.evaluate(async()=>{ + await new Promise(resolve=>requestAnimationFrame(()=>requestAnimationFrame(resolve))); + await Promise.race([Promise.all(document.getAnimations().map(animation=>animation.finished.catch(()=>{}))), + new Promise(resolve=>setTimeout(resolve,2000))]); +}); +const panelOf=page=>page.locator('#image-inspector'); + +/** + * The concept with every fixture image loaded through its own Load more (it loads 12 per batch, D37), as production + * has them: where the panel follows the grid (390px), reaching it would otherwise load the batch mid-comparison. + */ +async function openConceptPanel(page,theme) { + await openConcept(page,'images',theme);await page.mouse.move(0,0); + if(await page.locator(CARDS).count()<FIXTURE_IMAGES)await page.getByRole('button',{name:'Load more images',exact:true}).dispatchEvent('click'); + await expect(page.locator(CARDS)).toHaveCount(FIXTURE_IMAGES); + await page.evaluate(()=>{document.activeElement?.blur();window.scrollTo(0,0);}); + await expect(panelOf(page).locator('.panel-heading h2')).toHaveText(TITLE); + await settle(page); +} +async function openProductionPanel(page,theme) { + await openProduction(page,'images',theme);await page.mouse.move(0,0); + await expect(page.locator(CARDS).first()).toBeVisible(); + await expect(panelOf(page).locator('.panel-heading h2')).toHaveText(TITLE); + await expect(panelOf(page).locator('.detail-footer')).toBeVisible(); + await expect(panelOf(page).locator('.detail-inner > .tags')).toHaveCount(2); + await settle(page); +} + +/** The concept's tag chips in the open viewer as plain `span.chip`s (D20) and D34's coarse targets; returns an undo. */ +async function plainViewerChips(page) { + const restoreTargets=await mutate(page,{css:D34_COARSE_TARGETS}); + await page.evaluate(()=>{ + window.__plainChips=[...document.querySelectorAll('#viewer-details .tags > button.chip')].map(button=>{ + const span=document.createElement('span');span.className='chip';span.textContent=button.textContent; + button.replaceWith(span);return [span,button]; + }); + }); + await settle(page); + return async()=>{ + await page.evaluate(()=>{for(const [span,button] of window.__plainChips)span.replaceWith(button);delete window.__plainChips;}); + await restoreTargets(); + }; +} + +/** The controls an open "File & provenance" offers between its summary and the next disclosure's (m5). */ +async function fileControlsOf(page) { + const disclosure=page.locator('#viewer-details details').first(); + await disclosure.evaluate(node=>{node.open=true;});await settle(page); + const {controls}=(await inspectorOf(page)).viewerDetails; + await disclosure.evaluate(node=>{node.open=false;});await settle(page); + const names=controls.map(control=>control.name); + return controls.slice(names.indexOf('File & provenance')+1,names.indexOf('Workflow & raw metadata')).map(({tag,text,name})=>({tag,text,name})); +} + +/** + * The first card's viewer: its full details and open "File & provenance" measured (in the concept also with plain + * chips), then closed again. `mutation` (production) is applied once the viewer is open and undone before it closes. + */ +async function viewerOf(page,concept,mutation=null) { + await page.locator(CARDS).first().locator('.image-open').click(); + await expect(page.locator('#viewer-dialog[open] #viewer-details .panel-heading h2')).toHaveText(TITLE); + await settle(page); + const restoreMutation=mutation?await mutate(page,mutation):null; + if(mutation)await settle(page); + const {viewerDetails}=await inspectorOf(page); + const fileControls=await fileControlsOf(page); + let viewerPlain=null; + if(concept) {const restore=await plainViewerChips(page);viewerPlain=(await inspectorOf(page)).viewerDetails;await restore();} + await restoreMutation?.(); + await page.keyboard.press('Escape'); + await expect(page.locator('#viewer-dialog[open]')).toHaveCount(0); + // The concept re-renders the Images page on the viewer's `close` event (app.js updateImages): let it land first. + await page.evaluate(()=>new Promise(resolve=>setTimeout(resolve,0)));await settle(page); + return {viewerDetails,viewerPlain,fileControls}; +} +async function inventory(page,concept=false) { + const {panel}=await inspectorOf(page); + const {viewerDetails,viewerPlain,fileControls}=await viewerOf(page,concept); + return concept?{panel,viewerDetails,viewerPlain,fileControls}:{panel,viewerDetails,fileControls}; +} + +/** A ComfyUI host in a second tab that acknowledges what it applies (the shipped prompt bridge). */ +async function fakeHost(context) { + const host=await context.newPage();await host.goto('/prompt_manager/workspace#/overview'); + await host.evaluate(async()=>{ + const {createBridgeHost}=await import('/prompt_manager/js/shared/toolkit/prompt-bridge.js'); + window.hostValues={positive:'Existing positive',negative:'Existing negative'};window.applied=[]; + const target={id:'pair-1',label:'My prompt pair',type:'PromptManagerPair',roles:['positive','negative']}; + window.fixtureHost=createBridgeHost({origin:location.origin,namespace:'/',label:'Test ComfyUI',adapter:{ + listTargets:()=>[target], + preview:(_id,roles)=>({preview:{target,fields:Object.entries(roles).map(([role,after])=>({role,before:window.hostValues[role],after}))},snapshot:roles}), + apply:roles=>{Object.assign(window.hostValues,roles);window.applied.push(roles);return {targetId:target.id,values:roles};}, + }}); + }); + return host; +} + +/** Copy the positive: the concept's outcome is the toast, or its "Copy prompt" dialog where the clipboard is refused. */ +async function copyOf(page) { + // An earlier toast (the concept's Load more) is not the copy's outcome. + await panelOf(page).locator('.prompt-actions').first().scrollIntoViewIfNeeded();await settle(page); + await page.evaluate(()=>document.querySelectorAll('.toast').forEach(toast=>{toast.textContent='';})); + await panelOf(page).getByRole('button',{name:`Copy positive prompt: ${TITLE}`,exact:true}).click(); + const dialog=page.getByRole('dialog',{name:'Copy prompt',exact:true}); + await expect.poll(async()=>(await page.locator('.toast').textContent())||(await dialog.count()?'dialog':'')).not.toBe(''); + const result={toast:await page.locator('.toast').textContent(),dialog:await dialog.count()>0}; + if(result.dialog){await page.keyboard.press('Escape');await expect(dialog).toHaveCount(0);} + return result; +} + +/** The concept's send: its dialog previews both roles (app.js openSend). */ +async function conceptSendOf(page) { + await panelOf(page).getByRole('button',{name:`Send prompt pair to ComfyUI: ${TITLE}`,exact:true}).click(); + const modal=page.locator('dialog#modal[open]'); + await expect(modal.locator('#modal-title')).toHaveText('Send to ComfyUI'); + const previews=await modal.locator('.send-preview').evaluateAll(nodes=>nodes.map(node=>[node.querySelector('h3').textContent,node.querySelector('p').textContent])); + await page.keyboard.press('Escape');await expect(modal).toHaveCount(0); + return previews; +} + +/** Production's send: the acknowledged sender, previewing and applying both roles to a chosen node. */ +async function productionSendOf(page,context) { + const host=await fakeHost(context); + try { + await panelOf(page).getByRole('button',{name:`Send prompt pair to ComfyUI: ${TITLE}`,exact:true}).click(); + const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true});await expect(sender).toBeVisible(); + const windows=sender.getByRole('combobox',{name:'ComfyUI window',exact:true}); + await expect(windows.locator('option')).toContainText(['Test ComfyUI']);await windows.selectOption({label:'Test ComfyUI'}); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'My prompt pair'}); + await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); + await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click(); + await expect(sender).toContainText('Sent to ComfyUI.'); + const applied=await host.evaluate(()=>window.applied); + await sender.getByRole('button',{name:'Close',exact:true}).click();await expect(sender).toHaveCount(0); + return applied; + } finally {await host.evaluate(()=>window.fixtureHost.dispose());await host.close();} +} + +/** A tag chip: the collection it leaves (the count line), then the chip pressed again for the same result. */ +async function tagOf(page) { + const before=await page.locator('#result-count').textContent(); + await panelOf(page).locator('.detail-inner > .tags').last().getByRole('button',{name:'cinematic',exact:true}).click(); + await expect.poll(()=>page.locator('#result-count').textContent()).not.toBe(before); + await settle(page); + return {count:await page.locator('#result-count').textContent(), + cards:await page.locator(`${CARDS} .image-open`).evaluateAll(nodes=>nodes.map(node=>node.getAttribute('aria-label')))}; +} + +const historyOf=async page=>{ + await panelOf(page).getByRole('button',{name:'Prompt history',exact:true}).click(); + await expect.poll(()=>page.evaluate(()=>location.hash)).not.toMatch(/images$/); + return page.evaluate(()=>location.hash); +}; + +test.beforeEach(async({request},testInfo)=>{testInfo.manifest=await seedConcept(request,SEED);}); + +for(const [width,height] of SIZES)for(const theme of THEMES)test(`inspector panel geometry, tokens and controls match the concept ${width}x${height} ${theme}`,async({page,context,browserName},testInfo)=>{ + test.setTimeout(180_000); + // The concept's copy and production's both use the async clipboard; Chromium asks for the permission. + if(browserName==='chromium')await context.grantPermissions(['clipboard-read','clipboard-write']); + await page.setViewportSize({width,height}); + const manifest=testInfo.manifest; + const viewport={width,height}; + await openConceptPanel(page,theme); + await captureCanonicalState(page,testInfo,`concept-inspector-${width}-${theme}`); + const reference=await inventory(page,true); + const concept={copy:await copyOf(page),send:await conceptSendOf(page),tag:await tagOf(page)}; + await openConceptPanel(page,theme);concept.history=await historyOf(page); + + await openProductionPanel(page,theme); + await captureCanonicalState(page,testInfo,`production-inspector-${width}-${theme}`); + const actual=await inventory(page); + const production={copy:await copyOf(page),send:await productionSendOf(page,context)}; + const listing=page.waitForRequest(item=>item.url().includes('/images/output?')&&new URL(item.url()).searchParams.get('tags')==='cinematic'); + production.tag=await tagOf(page);await listing; + // D44: the applied tag is one active refinement, so the toolbar's Filters button counts it. + await page.getByRole('button',{name:'Filters · 1',exact:true}).click(); + production.tagFilter=await page.getByRole('dialog').getByRole('textbox',{name:'Tags',exact:true}).inputValue(); + await page.keyboard.press('Escape'); + await openProductionPanel(page,theme);production.history=await historyOf(page); + + const diff=compare(reference,actual,viewport); + const path=testInfo.outputPath('inspector-geometry.json'); + await writeFile(path,JSON.stringify({reference,actual,diff,concept,production},null,2)); + await testInfo.attach('inspector-geometry',{path,contentType:'application/json'}); + + // The concept itself is what this comparison assumes (spec §3.3, s523–615). + const {panel}=reference,coarse=await page.evaluate(()=>matchMedia('(pointer: coarse)').matches); + expect.soft(panel.width,'concept panel width').toBe(width>700?310:width-28); + expect.soft(panel.position,'concept panel position').toBe(width>700?'sticky':'static'); + expect.soft(panel.cover.height,'concept cover').toBe(width>700?150:200); + expect.soft(panel.inner.paddingTop,'concept inner padding').toBe('17px'); + expect.soft(panel.heading.h2.fontSize,'concept h2').toBe('14px'); + for(const section of panel.sections) { + expect.soft(section.label,`concept ${section.className} label`).toMatchObject({fontSize:'10px',letterSpacing:'0.7px',textTransform:'uppercase',fontWeight:'650'}); + expect.soft(section.text,`concept ${section.className} text`).toMatchObject({fontSize:'12px',lineHeight:'21px'}); + for(const button of section.actions.buttons)expect.soft(button,`concept ${section.className} action`).toMatchObject(coarse?{minHeight:'36px',minWidth:'36px'}:{minHeight:'27px',minWidth:'28px'}); + } + expect.soft(panel.sections.map(section=>section.label.color)[0],'concept role colors').not.toBe(panel.sections[1].label.color); + expect.soft(panel.facts,'concept facts').toMatchObject({columns:2,columnGap:'12px',rowGap:'12px',fontSize:width>700?'11px':'12px'}); + expect.soft(panel.footer,'concept footer').toMatchObject({paddingTop:'13px',paddingRight:'17px',paddingBottom:'13px',paddingLeft:'17px'}); + expect.soft(panel.badges.chips.map(chip=>chip.text),'concept badges').toEqual(['Execution recorded','Landscapes']); + expect.soft(reference.viewerDetails.structure.inner.slice(-2),'concept full details').toEqual(['details','details']); + + expect.soft(diff,'differences after the recorded deviations').toEqual([]); + // Copy: the concept's toast "Prompt copied." (or its dialog), in both documents. + expect.soft(production.copy,'copy').toEqual(concept.copy); + if(browserName==='chromium')expect.soft(production.copy,'copy toast').toEqual({toast:'Prompt copied.',dialog:false}); + // Send: the concept previews both roles; production applies exactly those texts and ComfyUI acknowledges them. + const [positive,negative]=concept.send; + expect.soft(concept.send.map(([heading])=>heading),'concept send').toEqual(['+ Positive · replace','− Negative · replace']); + expect.soft(production.send,'acknowledged send').toEqual([{positive:positive[1],negative:negative[1]}]); + expect.soft(negative[1],'fixture negative').toBe('blurry, low quality, low resolution'); + // Tag chip: D21, the Images Tags filter (the concept searches the tag's text instead), leaving the same images. + expect.soft(production.tagFilter,'tag chip filter').toBe('cinematic'); + // The concept's search also matches the forest prompt's text ("cinematic still"), which is not tagged cinematic + // (app.js 73, 115, 130): production leaves the tagged prompts' images, in the concept's order. + const tagged=concept.tag.cards.filter(card=>!card.startsWith('Open forest_')); + expect.soft(concept.tag.count,'concept tag search').toBe('12 of 12 images · 3 positive prompts'); + expect.soft(production.tag,'tag chip collection').toEqual({count:countCopy(`${tagged.length} of ${tagged.length} images · 2 positive prompts`),cards:tagged}); + // Prompt history: the concept's `#prompt/<id>` is production's Library history of the linked positive. + expect.soft(concept.history,'concept history').toBe('#prompt/alpine'); + expect.soft(production.history,'history').toBe(`#/library/${manifest.prompts[0].positive_id}`); +}); + +// D39: a recipe-linked image shows its recipe's own category and tags (#224), which the concept fixture's recipes do +// not have, so its panel has neither (the concept shows the prompt's for every image). +test('a recipe-linked image shows only its recipe metadata (D39)',async({page})=>{ + await page.setViewportSize({width:1600,height:1100}); + await openProductionPanel(page,'dark'); + await page.locator(CARDS).nth(1).locator('[aria-label^="Inspect"]').click(); + await expect(panelOf(page).locator('.panel-heading h2')).toHaveText('Quiet geometry'); + await expect(panelOf(page).locator('.detail-inner > .tags')).toHaveCount(1); + await expect(panelOf(page).locator('.detail-inner > .tags .chip')).toHaveText(['Execution recorded']); + await expect(panelOf(page).locator('.detail-footer button')).toHaveText(['Send to ComfyUI','Prompt history']); +}); + +// The comparison is only worth something if it notices a change (spec §7 mutation check): on the panel one CSS +// mutation (the role label's letter spacing) and one DOM mutation (the footer removed); in the viewer (final review m5) +// its pair actions removed and D38's Show in folder removed; each applied after an exact match. +test('inspector comparison detects mutations',async({page})=>{ + test.setTimeout(120_000); + const viewport={width:1600,height:1100}; + await page.setViewportSize(viewport); + await openConceptPanel(page,'dark'); + const reference=await inventory(page,true); + await openProductionPanel(page,'dark'); + const {viewerDetails,fileControls}=await inventory(page); + const measure=async()=>({panel:(await inspectorOf(page)).panel,viewerDetails,fileControls}); + assertInspectorMatch(reference,await measure(),viewport); + + const restoreCss=await mutate(page,{css:'.prompt-heading .role-label { letter-spacing: 0 }'}); + const cssMutated=await measure(); + expect(()=>assertInspectorMatch(reference,cssMutated,viewport)).toThrow(/panel\.sections\.0\.label\.letterSpacing/); + await restoreCss();await settle(page); + assertInspectorMatch(reference,await measure(),viewport); + + const panel=(await inspectorOf(page)).panel; + const viewerMutated=async mutation=>{const {viewerDetails,fileControls}=await viewerOf(page,false,mutation);return {panel,viewerDetails,fileControls};}; + const withoutActions=await viewerMutated({remove:['#viewer-details > button']}); + expect(()=>assertInspectorMatch(reference,withoutActions,viewport)).toThrow(/viewerDetails\.actions/); + const withoutReveal=await viewerMutated({remove:['#viewer-details button[aria-label$=" in folder"]']}); + expect(()=>assertInspectorMatch(reference,withoutReveal,viewport)).toThrow(/fileControls/); + assertInspectorMatch(reference,await viewerMutated(),viewport); + + await mutate(page,{remove:['#image-inspector > .detail-footer']}); + const domMutated=await measure(); + expect(()=>assertInspectorMatch(reference,domMutated,viewport)).toThrow(/panel\.footer/); +}); diff --git a/tests/browser/prompt-inspector.spec.mjs b/tests/browser/prompt-inspector.spec.mjs index 286a95a..9ebca0e 100644 --- a/tests/browser/prompt-inspector.spec.mjs +++ b/tests/browser/prompt-inspector.spec.mjs @@ -75,9 +75,10 @@ test('physical image inspector resolves positive and negative metadata separatel test('explicit empty negative is sendable while unrecorded text is never invented',async({page})=>{ await page.goto(workspace);await select(page,'0:empty-negative.png'); await expect(role(details(page),'Negative').locator('p.prompt-text')).toHaveText('No negative prompt (explicitly empty)'); - await expect(details(page).getByRole('button',{name:'Send negative prompt to ComfyUI',exact:true})).toBeEnabled(); + await expect(details(page).getByRole('button',{name:/^Send negative prompt to ComfyUI: /})).toBeEnabled(); await select(page,'0:uncertain.png');await expect(details(page)).toContainText('Not recorded in this file.'); - await expect(details(page).getByRole('button',{name:/^Send (positive|negative) prompt to ComfyUI$/})).toHaveCount(0); + // Every inspector send is named after its prompt ("…to ComfyUI: <title>"); an untitled one ends at "ComfyUI". + await expect(details(page).getByRole('button',{name:/^Send (positive|negative) prompt to ComfyUI(: |$)/})).toHaveCount(0); }); test('inspection failures retry and a delayed previous selection cannot replace the current prompt',async({page})=>{ @@ -157,7 +158,7 @@ test('ComfyUI ↗ opens the pair send for a selected resolved image, else follow test('host apply rejection retains the current text and requires a new preview',async({page,context})=>{ const host=await fakeHost(context,page,{rejectApply:true}); try { - await page.goto(workspace);await select(page,'0:shared/same.png');await details(page).getByRole('button',{name:'Send positive prompt to ComfyUI',exact:true}).click(); + await page.goto(workspace);await select(page,'0:shared/same.png');await details(page).getByRole('button',{name:/^Send positive prompt to ComfyUI: /}).click(); const sender=await chooseDestination(page);await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click();await expect(sender).toContainText(/Workflow changed/); await expect(sender.getByRole('button',{name:'Apply to ComfyUI',exact:true})).toBeDisabled(); @@ -169,7 +170,7 @@ test('nested viewer sender Escape restores focus without closing the image and n await page.goto(workspace);await select(page,'0:shared/same.png'); await page.locator('.media-card.selected .image-open').click(); const viewer=page.locator('dialog[open]').filter({has:page.locator('.viewer-canvas')});await expect(viewer).toBeVisible(); - const send=viewer.getByRole('button',{name:'Send positive prompt to ComfyUI',exact:true});await send.click();await expect(dialog(page)).toBeVisible(); + const send=viewer.getByRole('button',{name:/^Send positive prompt to ComfyUI: /});await send.click();await expect(dialog(page)).toBeVisible(); await page.keyboard.press('Escape');await expect(dialog(page)).toBeHidden();await expect(viewer).toBeVisible();await expect(send).toBeFocused(); await send.click();await page.evaluate(()=>{location.hash='#/overview';});await expect(page.locator('dialog[open]')).toHaveCount(0); }); @@ -178,7 +179,7 @@ test('empty negative send clears only the negative destination and preserves its const host=await fakeHost(context,page); try { await page.goto(workspace);await select(page,'0:empty-negative.png'); - await details(page).getByRole('button',{name:'Send negative prompt to ComfyUI',exact:true}).click(); + await details(page).getByRole('button',{name:/^Send negative prompt to ComfyUI: /}).click(); const sender=await chooseDestination(page);await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click();await expect(sender).toContainText('Sent to ComfyUI.'); expect(await host.evaluate(()=>window.hostValues)).toEqual({positive:'Existing positive',negative:''}); @@ -187,7 +188,7 @@ test('empty negative send clears only the negative destination and preserves its test('no open ComfyUI window is actionable and rediscovery finds a newly opened host',async({page,context})=>{ await page.goto(workspace);await select(page,'0:shared/same.png'); - await details(page).getByRole('button',{name:'Send positive prompt to ComfyUI',exact:true}).click(); + await details(page).getByRole('button',{name:/^Send positive prompt to ComfyUI: /}).click(); const sender=dialog(page);await expect(sender).toContainText('No compatible ComfyUI windows found.'); const host=await fakeHost(context,page); try { @@ -205,11 +206,11 @@ test('mobile inspector exposes full long prompts, truthful uncertainty and copy await page.setViewportSize({width:390,height:844});await page.goto(workspace);await select(page,'0:shared/same.png'); const positive=role(details(page),'Positive');await expect(positive.locator('p.prompt-text')).toHaveText(text); await expect(role(details(page),'Negative')).toContainText('Multiple possible prompts. Source could not be determined.'); - await expect(details(page).getByRole('button',{name:'Send negative prompt to ComfyUI',exact:true})).toHaveCount(0); - await positive.getByRole('button',{name:'Copy positive prompt',exact:true}).click();await expect(page.getByRole('dialog',{name:'Copy prompt',exact:true})).toBeVisible(); + await expect(details(page).getByRole('button',{name:/^Send negative prompt to ComfyUI: /})).toHaveCount(0); + await positive.getByRole('button',{name:/^Copy positive prompt: /}).click();await expect(page.getByRole('dialog',{name:'Copy prompt',exact:true})).toBeVisible(); await page.getByRole('button',{name:'Close dialog',exact:true}).click(); expect(await page.evaluate(()=>document.documentElement.scrollWidth<=innerWidth+1)).toBe(true); - await positive.getByRole('button',{name:'Send positive prompt to ComfyUI',exact:true}).click(); + await positive.getByRole('button',{name:/^Send positive prompt to ComfyUI: /}).click(); await expect(dialog(page)).toBeVisible();const bounds=await dialog(page).boundingBox(); expect(bounds.x).toBeGreaterThanOrEqual(0);expect(bounds.x+bounds.width).toBeLessThanOrEqual(391); }); @@ -284,8 +285,8 @@ test('prompt role headings contain the concept label and compact real actions wi const block=role(details(page),name),heading=block.locator('.prompt-heading'); await expect(block).toHaveClass(new RegExp(`^prompt-section ${name.toLowerCase()}$`)); await expect(heading.locator('span.role-label')).toHaveText(label); - await expect(heading.locator('.prompt-actions button.icon-btn').first()).toHaveAccessibleName(`Copy ${name.toLowerCase()} prompt`); - await expect(heading.locator('.prompt-actions button.send.icon-btn')).toHaveAccessibleName(`Send ${name.toLowerCase()} prompt to ComfyUI`); + await expect(heading.locator('.prompt-actions button.icon-btn').first()).toHaveAccessibleName(new RegExp(`^Copy ${name.toLowerCase()} prompt: same\\.png$`)); + await expect(heading.locator('.prompt-actions button.send.icon-btn')).toHaveAccessibleName(new RegExp(`^Send ${name.toLowerCase()} prompt to ComfyUI: same\\.png$`)); await expect(heading.locator('[data-copy-role] svg')).toHaveCount(1); expect(await heading.evaluate(node=>node.nextElementSibling.matches('p.prompt-text')&&!node.nextElementSibling.nextElementSibling)).toBe(true); } @@ -301,7 +302,7 @@ test('copy toast and fallback dialog',async({page})=>{ await page.addInitScript(()=>{window.copied=[];window.clipboardFails=false;Object.defineProperty(navigator,'clipboard',{configurable:true,value:{writeText:async text=>{if(window.clipboardFails)throw new Error('Permission denied');window.copied.push(text);}}});}); await page.goto(workspace);await select(page,'0:shared/same.png'); const positive=role(details(page),'Positive'),text=await positive.locator('p.prompt-text').textContent(); - const copy=positive.getByRole('button',{name:'Copy positive prompt',exact:true});await copy.click(); + const copy=positive.getByRole('button',{name:/^Copy positive prompt: /});await copy.click(); await expect(page.locator('#workspace-toast')).toHaveText('Prompt copied.');expect(await page.evaluate(()=>window.copied)).toEqual([text]); await expect(positive.getByRole('status')).toHaveCount(0); await page.evaluate(()=>{window.clipboardFails=true;});await copy.click(); diff --git a/tests/browser/server.py b/tests/browser/server.py index 26066f2..53333bc 100644 --- a/tests/browser/server.py +++ b/tests/browser/server.py @@ -17,6 +17,7 @@ from PIL.PngImagePlugin import PngInfo from tests.browser.workspace_concept_fixture import ( MAX_EXTRA_IMAGES as CONCEPT_MAX_EXTRA_IMAGES, MAX_SEEDLESS_IMAGES as CONCEPT_MAX_SEEDLESS_IMAGES, + MAX_POSITIVE_LINKS as CONCEPT_MAX_POSITIVE_LINKS, seed_workspace_concept, ) from tests.browser.workspace_visual_fixture import seed_workspace_visual @@ -76,6 +77,53 @@ class TestModel: return ["generated", Path(image_path).parent.parent.name] +REVEAL_OUTCOMES = ("opened", "unavailable", "fail") + + +def register_reveal_stub(routes, api, root: Path): + """D38: Show in folder never opens a real file manager here. + + The API's opener records the resolved path; ``POST /__test__/reveal`` + sets the next outcome and ``GET /__test__/reveal`` lists the opened paths. + """ + from utils.file_reveal import OpenerUnavailable, RevealGate + + state = {"outcome": "opened", "opened": []} + + def opener(path: Path, gallery_root: Path) -> None: + if state["outcome"] == "unavailable": + raise OpenerUnavailable() + if state["outcome"] == "fail": + raise OSError("stubbed start failure") + state["opened"].append(Path(path).relative_to(root.resolve()).as_posix()) + + api._reveal_opener = opener + # Each click gets an answer here; the cooldown is a unit-tested default. + api._reveal_gate = RevealGate(cooldown=0) + + @routes.post("/__test__/reveal") + async def set_reveal_outcome(request: web.Request) -> web.Response: + data = await request.json() if request.can_read_body else None + if ( + not isinstance(data, dict) + or set(data) != {"outcome"} + or data["outcome"] not in REVEAL_OUTCOMES + ): + raise web.HTTPBadRequest() + state["outcome"] = data["outcome"] + return web.json_response({"success": True}) + + @routes.get("/__test__/reveal") + async def reveal_calls(request: web.Request) -> web.Response: + return web.json_response({"opened": state["opened"]}) + + def reset() -> None: + state["outcome"] = "opened" + state["opened"].clear() + + return types.SimpleNamespace(reset=reset) + + def create_app(root: Path) -> web.Application: os.environ["PROMPT_MANAGER_CONFIG_PATH"] = str(root / "config.json") os.chdir(root) @@ -111,6 +159,7 @@ 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) + reveal_stub = register_reveal_stub(routes, api, root) async def canonical_asset(request: web.Request) -> web.Response: filename, content_type = CANONICAL_ASSETS[request.path] @@ -311,6 +360,7 @@ def create_app(root: Path) -> web.Application: history_fixture = manifest return web.json_response({"success": True, **manifest}) history_commands.reset() + reveal_stub.reset() workspace_visual = data.get("workspace_visual", False) full_inspector = data.get("full_inspector", False) extra = data.get("extra_images", 0) @@ -393,6 +443,7 @@ def create_app(root: Path) -> web.Application: limits = { "extra_images": CONCEPT_MAX_EXTRA_IMAGES, "seedless_images": CONCEPT_MAX_SEEDLESS_IMAGES, + "positive_links": CONCEPT_MAX_POSITIVE_LINKS, } if ( request.query @@ -408,6 +459,7 @@ def create_app(root: Path) -> web.Application: raise web.HTTPBadRequest() history_commands.reset() library_editor_commands.reset() + reveal_stub.reset() history_fixture = None manifest = await api._run_in_executor( lambda: seed_workspace_concept( @@ -415,6 +467,7 @@ def create_app(root: Path) -> web.Application: root, extra_images=data.get("extra_images", 0), seedless_images=data.get("seedless_images", 0), + positive_links=data.get("positive_links", 0), ) ) return web.json_response({"success": True, **manifest}) diff --git a/tests/browser/workspace-history.spec.mjs b/tests/browser/workspace-history.spec.mjs index d79183c..febf0b8 100644 --- a/tests/browser/workspace-history.spec.mjs +++ b/tests/browser/workspace-history.spec.mjs @@ -474,7 +474,7 @@ test('the inline current file names its prompts apart from the frozen run fields const current=dialog(page).getByRole('region',{name:'Current reported file',exact:true});await expect(current).toBeVisible(); await expect(current.getByRole('region',{name:'Current file positive prompt',exact:true}).locator('p.prompt-text')).toHaveText('current file positive'); await expect(current.getByRole('region',{name:'Current file negative prompt',exact:true}).locator('p.prompt-text')).toHaveText('current file negative'); - await expect(current.getByRole('button',{name:'Copy current file positive prompt',exact:true})).toBeVisible(); + await expect(current.getByRole('button',{name:/^Copy current file positive prompt: /})).toBeVisible(); for(const role of ['Positive','Negative'])await expect(dialog(page).getByRole('region',{name:`${role} prompt`,exact:true})).toHaveCount(1); await expect(current.getByRole('region',{name:/^(Positive|Negative) prompt$/})).toHaveCount(0); expect(await exact(dialog(page).getByRole('region',{name:'Positive prompt',exact:true}).locator('p.prompt-text'))).toBe(before); diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index f32f745..b5ce950 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1569,6 +1569,63 @@ test('unreachable sources and categories announce one error', async ({ page }) = await expect(page.getByRole('alert')).toHaveCount(0); }); +// PR D-b Task 3 (D42): the shared toast keeps the concept's fixed bottom placement, so a discovery alert that a rescan +// leaves visible must not stay under it. The window ends just below the alert, inside the toast's band, as on a short +// window or a phone with its browser bars shown. +const boxOf = locator => locator.evaluate(node => { + const { top, right, bottom, left } = node.getBoundingClientRect(); + return { top, right, bottom, left }; +}); +const overlaps = (a, b) => a.left < b.right && b.left < a.right && a.top < b.bottom && b.top < a.bottom; +for (const width of [1600, 390]) { + test(`Rescan toast does not cover the discovery alert it leaves at ${width}px`, async ({ page }) => { + await page.setViewportSize({ width, height: 1100 }); + await page.route('**/prompt_manager/library/options*', route => route.abort('failed')); + await page.goto(workspace); + await expect(cards(page)).toHaveCount(2); + const alert = page.getByRole('alert'); + const failed = 'Categories could not be loaded. Rescan from Import to retry.'; + await expect(alert).toHaveText(failed); + const toastInset = width > 700 ? 22 : 12; + await page.setViewportSize({ width, height: Math.ceil((await boxOf(alert)).bottom) + toastInset + 16 }); + await rescan(page); + await expect(toast(page)).toHaveText(/^Rescan finished\. /); + await expect(alert).toHaveText(failed); + await expect.poll(async () => overlaps(await boxOf(alert), await boxOf(toast(page)))).toBe(false); + const [shown, placed] = [await boxOf(alert), await boxOf(toast(page))]; + expect(shown.top).toBeGreaterThanOrEqual(0); + expect(shown.bottom).toBeLessThanOrEqual(placed.top); + // The concept's toast stays where the concept puts it: centred, its inset from the bottom. + const viewport = page.viewportSize(); + expect(Math.round(viewport.height - placed.bottom)).toBe(toastInset); + expect(Math.abs((placed.left + placed.right) / 2 - viewport.width / 2)).toBeLessThan(1); + }); +} + +// Final review m3: D42 scrolls only when the alert meets the toast's band, never back up to one above the window. +test('Rescan does not scroll back to a discovery alert above the window', async ({ page }) => { + await page.setViewportSize({ width: 1600, height: 1100 }); + await page.route('**/prompt_manager/library/options*', route => route.abort('failed')); + await page.goto(workspace); + await expect(cards(page)).toHaveCount(2); + const alert = page.getByRole('alert'); + await expect(alert).toHaveText('Categories could not be loaded. Rescan from Import to retry.'); + await page.setViewportSize({ width: 1600, height: 260 }); + // Import is at the top; the page is scrolled down to the last card while its dialog is open. + await page.getByRole('button', { name: 'Import', exact: true }).click(); + await cards(page).last().evaluate(node => node.scrollIntoView({ block: 'end' })); + expect((await boxOf(alert)).bottom).toBeLessThan(0); + // The reload itself may move the page; what D42 must not do is scroll the alert back into view. + await alert.evaluate(node => { + window.alertScrolls = 0; + node.scrollIntoView = (...args) => { window.alertScrolls += 1; Element.prototype.scrollIntoView.apply(node, args); }; + }); + await importDialog(page).getByRole('button', { name: 'Rescan', exact: true }).click(); + await expect(toast(page)).toHaveText(/^Rescan finished\. /); + await expect(alert).toBeAttached(); + expect(await page.evaluate(() => window.alertScrolls)).toBe(0); +}); + // #249 final review m8: D36's alphabetical categories hold whatever order the server sends. test('toolbar categories are alphabetical regardless of server order', async ({ page }) => { await page.route('**/prompt_manager/library/options*', route => route.fulfill({ diff --git a/tests/browser/workspace-visual.spec.mjs b/tests/browser/workspace-visual.spec.mjs index 2cb0dc3..e30f304 100644 --- a/tests/browser/workspace-visual.spec.mjs +++ b/tests/browser/workspace-visual.spec.mjs @@ -125,11 +125,11 @@ for (const [width, height] of [[1600, 1100], [390, 844]]) for (const theme of [' const viewer = page.locator('dialog#viewer-dialog');await expect(viewer).toBeVisible(); await expect(viewer.getByRole('region', { name: 'Positive prompt', exact: true })).toContainText(fixture.canonical_positive); await expect(viewer.getByRole('region', { name: 'Negative prompt', exact: true })).toContainText(fixture.canonical_negative); - for (const name of ['Copy positive prompt', 'Send positive prompt to ComfyUI', 'Copy negative prompt', 'Send negative prompt to ComfyUI']) await expect(viewer.getByRole('button', { name, exact: true })).toBeVisible(); + for (const name of ['Copy positive prompt', 'Send positive prompt to ComfyUI', 'Copy negative prompt', 'Send negative prompt to ComfyUI']) await expect(viewer.getByRole('button', { name: new RegExp(`^${name}: `) })).toBeVisible(); await expect(viewer.getByRole('button', { name: /^Send current file prompt pair to ComfyUI: |^Send prompt pair to ComfyUI: / })).toBeVisible(); await expect(viewer).toContainText('42001');await expect(viewer).toContainText('720');await expect(viewer).toContainText('480'); const currentPositive = viewer.getByRole('region', { name: 'Positive prompt', exact: true }); - await currentPositive.getByRole('button', { name: 'Copy positive prompt', exact: true }).click(); + await currentPositive.getByRole('button', { name: /^Copy positive prompt: / }).click(); // The concept's copy outcome: the shared toast, or its "Copy prompt" dialog when the clipboard is refused. const copyFallback = page.getByRole('dialog', { name: 'Copy prompt', exact: true }); await expect(page.locator('#workspace-toast').filter({ hasText: 'Prompt copied.' }).or(copyFallback)).toHaveCount(1); diff --git a/tests/browser/workspace_concept_fixture.py b/tests/browser/workspace_concept_fixture.py index 66c3da5..ef9d4a5 100644 --- a/tests/browser/workspace_concept_fixture.py +++ b/tests/browser/workspace_concept_fixture.py @@ -157,6 +157,9 @@ CONCEPT_MODEL = "illustrative-model.safetensors" MAX_EXTRA_IMAGES = 120 # Unlinked, seedless images (D11 "W × H · PNG") newer than the concept's, for D3 checks. MAX_SEEDLESS_IMAGES = 2 +# The newest images linked to their positive alone (no recipe), for the inspector's +# positive-scope category and tag chips (D39). +MAX_POSITIVE_LINKS = 6 SEEDLESS_SIZE = (1536, 1152) SESSION = "5c1f9a7e-3f0b-4a55-9d0e-2b8c4f6a7d10" _TABLES = ( @@ -341,6 +344,7 @@ def seed_workspace_concept( *, extra_images: int = 0, seedless_images: int = 0, + positive_links: int = 0, ) -> dict: """Replace this server's disposable rows and files with the concept's data. @@ -350,6 +354,7 @@ def seed_workspace_concept( """ _bounded("extra_images", extra_images, MAX_EXTRA_IMAGES) _bounded("seedless_images", seedless_images, MAX_SEEDLESS_IMAGES) + _bounded("positive_links", positive_links, MAX_POSITIVE_LINKS) _clear(api, root) db = api.db epoch = db.get_execution_epoch() @@ -405,8 +410,9 @@ def seed_workspace_concept( os.utime(path, (stamp, stamp)) owner = prompts[prompt_index] image_id = db.link_image_to_prompt(owner["positive_id"], str(path)) - pair_id = owner["pair_ids"][variant] - db.link_image_to_pair(pair_id, image_id, {"source": "concept fixture"}) + pair_id = None if index < positive_links else owner["pair_ids"][variant] + if pair_id is not None: + db.link_image_to_pair(pair_id, image_id, {"source": "concept fixture"}) if concept_starred(prompt_index, round_index): db.set_image_favorite( favorite_root, _favorite_relative_key(f"concepts/{name}") diff --git a/tests/js/prompt-blocks.test.js b/tests/js/prompt-blocks.test.js index 87a6098..15f353b 100644 --- a/tests/js/prompt-blocks.test.js +++ b/tests/js/prompt-blocks.test.js @@ -144,6 +144,20 @@ test('a context label names nested prompt regions and actions apart from the fro }finally{scope.dispose();f.restore();} }); +test('a title names each role action after its prompt as the concept does',()=>{ + const f=dom(),scope=createScope(),container=new f.Element('div'); + try{ + module.mountPromptBlocks(container,{parentScope:scope,sender:{open:()=>{}},title:'Alpine observatory',prompts:{positive:record('p'),negative:record('n')}}); + const actions=f.walk(container,node=>node.tagName==='button').map(node=>node.attributes['aria-label']); + assert.deepEqual(actions,['Copy positive prompt: Alpine observatory','Send positive prompt to ComfyUI: Alpine observatory', + 'Copy negative prompt: Alpine observatory','Send negative prompt to ComfyUI: Alpine observatory','Send prompt pair']); + const nested=new f.Element('div'); + module.mountPromptBlocks(nested,{parentScope:scope,sender:{open:()=>{}},contextLabel:'Current file',title:'Alpine observatory',includePairAction:false,prompts:{positive:record('p')}}); + assert.deepEqual(f.walk(nested,node=>node.tagName==='button').map(node=>node.attributes['aria-label']), + ['Copy current file positive prompt: Alpine observatory','Send current file positive prompt to ComfyUI: Alpine observatory']); + }finally{scope.dispose();f.restore();} +}); + test('a title names each action after its prompt (concept "Copy positive prompt: <title>") while regions keep their names',()=>{ const f=dom(),scope=createScope(),container=new f.Element('div'); try{ diff --git a/tests/js/workspace-file-inspector.test.js b/tests/js/workspace-file-inspector.test.js index d129814..3a01d7a 100644 --- a/tests/js/workspace-file-inspector.test.js +++ b/tests/js/workspace-file-inspector.test.js @@ -65,3 +65,24 @@ test('limited workflow record exports its representation and disables workflow d assert.equal(downloads.workflow.available,false);assert.equal(downloads.workflow.bodyText,null); assert.equal(downloads.workflow.reason,'Workflow exceeds inspection limits. Download the original file to inspect it externally.'); }); + +test('Show in folder needs the full listing identity (D38)',async()=>{ + const {revealTarget}=await load(),scope='a'.repeat(64); + assert.deepEqual(revealTarget(record().identity),{root_index:2,root_scope:scope,path:'a.png'}); + for(const identity of [ + {kind:'rooted-file',root_index:2,path:'a.png',root_scope:null}, + {kind:'rooted-file',root_index:'2',path:'a.png',root_scope:scope}, + {kind:'rooted-file',root_index:2,path:'',root_scope:scope}, + {kind:'reported-location',path:'folder/a.png',current_scope:scope}, + null, + ])assert.equal(revealTarget(identity),null); +}); +test('Show in folder outcomes use the D38 toasts',async()=>{ + const {revealOutcome}=await load(); + assert.equal(revealOutcome(null),'Opened the folder on this computer.'); + assert.equal(revealOutcome({status:403,code:'not-local'}),'Show in folder works only in a browser on the computer running ComfyUI.'); + assert.equal(revealOutcome({status:501,code:'unavailable'}),'No file manager is available on this computer.'); + assert.equal(revealOutcome({status:429,code:'busy'}),'The folder is already opening.'); + for(const error of [{status:403,code:null},{status:409,code:'source-changed'},{status:415,code:'unsupported-media-type'},{status:429,code:null},{status:500},new TypeError('offline')]) + assert.equal(revealOutcome(error),'The folder could not be opened. Try again.'); +}); diff --git a/tests/test_image_reveal.py b/tests/test_image_reveal.py new file mode 100644 index 0000000..4b4b050 --- /dev/null +++ b/tests/test_image_reveal.py @@ -0,0 +1,919 @@ +"""Show in folder (TESTING.md D38): the local-only reveal route and its openers. + +No test launches a file manager: ``subprocess.Popen``, ``sys.platform`` and +``shutil.which`` are mocked, and route tests swap the API's opener. +""" + +import ctypes +import json +import logging +import os +import subprocess +import sys +import tempfile +import types +import unittest +from types import SimpleNamespace +from pathlib import Path +from unittest.mock import MagicMock, PropertyMock, patch + +from aiohttp import web +from multidict import CIMultiDict + +try: + from test_api_images import ImageAPITestCase, make_png +except ImportError: + from tests.test_api_images import ImageAPITestCase, make_png + +import utils.file_reveal as file_reveal +from utils.file_reveal import ( + OpenerUnavailable, + is_local_request, + reveal_in_file_manager, +) + +REVEAL = "/prompt_manager/images/reveal" +PROXY_HEADERS = ( + "Forwarded", + "Via", + "X-Forwarded-For", + "X-Forwarded-Host", + "X-Forwarded-Proto", + "X-Forwarded-Port", + "X-Forwarded-Server", + "X-Real-IP", + "X-Original-Forwarded-For", + "X-Original-Host", + "CF-Connecting-IP", + "True-Client-IP", + "X-Client-IP", + "X-Cluster-Client-IP", + "Fastly-Client-IP", +) +ORIGIN = "http://127.0.0.1:8188" + + +def headers(**values): + base = {"Host": "127.0.0.1:8188", "Origin": ORIGIN} + base.update({key.replace("_", "-"): value for key, value in values.items()}) + return CIMultiDict({key: value for key, value in base.items() if value is not None}) + + +class TestIsLocalRequest(unittest.TestCase): + def test_same_origin_loopback_clients_by_loopback_name_are_local(self): + for remote, host, origin in ( + ("127.0.0.1", "127.0.0.1:8188", ORIGIN), + ("127.8.9.10", "localhost:8188", "http://localhost:8188"), + ("::1", "[::1]:8188", "http://[::1]:8188"), + ("::ffff:127.0.0.1", "comfy.localhost", "http://comfy.localhost"), + ("127.0.0.1", "LOCALHOST:8188", "http://localhost:8188"), + ("127.0.0.1", "localhost", "http://localhost:80"), + ): + with self.subTest(remote=remote, host=host): + self.assertTrue( + is_local_request(remote, headers(Host=host, Origin=origin)) + ) + self.assertTrue( + is_local_request( + "127.0.0.1", + headers(Host="localhost", Origin="https://localhost:443"), + "https", + ) + ) + + def test_non_loopback_or_unknown_remote_is_not_local(self): + for remote in ("192.168.1.5", "10.0.0.1", "::2", "fe80::1%eth0", "", None): + with self.subTest(remote=remote): + self.assertFalse(is_local_request(remote, headers())) + + def test_any_forwarding_header_makes_loopback_remote(self): + for name in (*PROXY_HEADERS, "X-Forwarded-Anything", "X-Original-URI"): + for spelling in (name, name.lower()): + with self.subTest(header=spelling): + request_headers = headers() + request_headers[spelling] = "127.0.0.1" + self.assertFalse(is_local_request("127.0.0.1", request_headers)) + + def test_origin_is_required_and_must_be_the_request_origin(self): + # A header-less proxy or curl leaves Origin out; a page on another + # loopback port or scheme sends its own (security review M1, L2). + self.assertFalse(is_local_request("127.0.0.1", headers(Origin=None))) + for origin in ( + "", + "null", + "http://localhost:3000", + "http://127.0.0.1:3000", + "http://127.0.0.1", + "https://127.0.0.1:8188", + "http://localhost:8188", + "http://foo.localhost:8188", + "http://evil.example:8188", + "http://[bad", + "http://127.0.0.1:8188/path", + "http://user@127.0.0.1:8188", + "file://127.0.0.1:8188", + ): + with self.subTest(origin=origin): + self.assertFalse(is_local_request("127.0.0.1", headers(Origin=origin))) + self.assertFalse(is_local_request("127.0.0.1", headers(Origin=ORIGIN), "https")) + + def test_sec_fetch_site_must_be_same_origin_when_sent(self): + self.assertTrue( + is_local_request("127.0.0.1", headers(Sec_Fetch_Site="same-origin")) + ) + for site in ("same-site", "cross-site", "none", ""): + with self.subTest(site=site): + self.assertFalse( + is_local_request("127.0.0.1", headers(Sec_Fetch_Site=site)) + ) + + def test_foreign_host_is_not_local(self): + # A DNS-rebinding page reaches 127.0.0.1 under its own name. + for host in ("evil.example:8188", "", "[bad", "127.0.0.1:bad"): + with self.subTest(host=host): + self.assertFalse( + is_local_request( + "127.0.0.1", headers(Host=host, Origin=f"http://{host}") + ) + ) + self.assertFalse(is_local_request("127.0.0.1", CIMultiDict({"Origin": ORIGIN}))) + + +class TestRevealGate(unittest.TestCase): + def test_one_reveal_at_a_time_then_a_cooldown(self): + now = [100.0] + gate = file_reveal.RevealGate(cooldown=1.0, clock=lambda: now[0]) + self.assertTrue(gate.try_enter()) + self.assertFalse(gate.try_enter()) + now[0] += 5 + self.assertFalse(gate.try_enter()) + gate.leave() + now[0] += 0.5 + self.assertFalse(gate.try_enter()) + now[0] += 0.5 + self.assertTrue(gate.try_enter()) + + def test_zero_cooldown_only_blocks_while_busy(self): + gate = file_reveal.RevealGate(cooldown=0) + self.assertTrue(gate.try_enter()) + self.assertFalse(gate.try_enter()) + gate.leave() + self.assertTrue(gate.try_enter()) + + +class TestRevealInFileManager(unittest.TestCase): + def setUp(self): + temp = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + self.addCleanup(temp.cleanup) + self.root = Path(os.path.realpath(temp.name)) / "out" + self.path = self.root / "sub" / "a.png" + self.path.parent.mkdir(parents=True) + self.path.write_bytes(b"png") + self.popen = patch.object(subprocess, "Popen").start() + self.reaped = patch.object(file_reveal, "_reap").start() + self.addCleanup(patch.stopall) + + def reveal(self, path=None): + reveal_in_file_manager(self.path if path is None else path, self.root) + + def which(self, available): + def lookup(name, path=None): + self.assertEqual(path, file_reveal.SYSTEM_PATH) + return available.get(name) + + return patch.object(file_reveal.shutil, "which", side_effect=lookup) + + def argv(self, call=0): + return self.popen.call_args_list[call].args[0] + + def assert_detached(self, call=0): + options = self.popen.call_args_list[call].kwargs + for stream in ("stdin", "stdout", "stderr"): + self.assertIs(options[stream], subprocess.DEVNULL) + self.assertIs(options["close_fds"], True) + self.assertIs(options["start_new_session"], True) + self.assertNotIn("shell", options) + self.assertEqual(options["env"]["PATH"], file_reveal.SYSTEM_PATH) + + def windows(self, selected=True, startfile=None): + """Windows without Windows: the shell libraries and os.startfile + injected; the patched shell records its calls.""" + shell = FakeShell(select=0 if selected else -2147467259) + select = patch.object( + file_reveal, + "_windows_libraries", + return_value=(shell.ole32, shell.shell32), + ) + select.shell = shell + start = patch.object( + file_reveal.os, "startfile", startfile or MagicMock(), create=True + ) + platform = patch.object(file_reveal.sys, "platform", "win32") + return select, start, platform + + def test_windows_selects_through_the_shell_api_without_a_process(self): + # A comma path splits Explorer's own command line (security review H1); + # the shell API takes the path as one value, unchanged. + path = self.root / "a," / "b,c.png" + path.parent.mkdir() + path.write_bytes(b"png") + select, start, platform = self.windows(selected=True) + with select, start as startfile, platform: + self.reveal(path) + self.assertEqual(dict(select.shell.calls)["ILCreateFromPathW"], (str(path),)) + startfile.assert_not_called() + self.popen.assert_not_called() + + def test_windows_falls_back_to_opening_the_folder(self): + select, start, platform = self.windows(selected=False) + with select, start as startfile, platform: + self.reveal() + self.assertEqual( + dict(select.shell.calls)["ILCreateFromPathW"], (str(self.path),) + ) + startfile.assert_called_once_with(str(self.path.parent), "explore") + self.popen.assert_not_called() + + def test_windows_folder_start_failure_raises(self): + select, start, platform = self.windows( + selected=False, startfile=MagicMock(side_effect=OSError("denied")) + ) + with select, start, platform, self.assertRaises(OSError): + self.reveal() + self.popen.assert_not_called() + + def test_windows_without_startfile_is_unavailable(self): + with ( + patch.object(file_reveal, "_windows_libraries", side_effect=OSError), + patch.object(file_reveal.sys, "platform", "win32"), + patch.object(file_reveal.os, "startfile", None, create=True), + self.assertRaises(OpenerUnavailable), + ): + self.reveal() + self.popen.assert_not_called() + + def test_macos_reveals_with_the_system_open(self): + with ( + patch.object(file_reveal.sys, "platform", "darwin"), + patch.object(file_reveal, "_macos_open", return_value="/usr/bin/open"), + self.which({"open": "/planted/open"}), + ): + self.reveal() + self.assertEqual(self.argv(), ["/usr/bin/open", "-R", str(self.path)]) + self.assert_detached() + self.popen.return_value.wait.assert_not_called() + self.reaped.assert_called_once_with(self.popen.return_value) + + def test_macos_open_is_the_fixed_system_path(self): + self.assertEqual(file_reveal.MACOS_OPEN, "/usr/bin/open") + with patch.object(file_reveal.os.path, "isfile", return_value=False): + self.assertIsNone(file_reveal._macos_open()) + with ( + patch.object(file_reveal.os.path, "isfile", return_value=True), + patch.object(file_reveal.os, "access", return_value=True), + ): + self.assertEqual(file_reveal._macos_open(), "/usr/bin/open") + with ( + patch.object(file_reveal.sys, "platform", "darwin"), + patch.object(file_reveal, "_macos_open", return_value=None), + self.assertRaises(OpenerUnavailable), + ): + self.reveal() + self.popen.assert_not_called() + + def test_relative_path_lookup_never_counts(self): + with ( + patch.object(file_reveal.sys, "platform", "linux"), + self.which({"dbus-send": "./dbus-send", "xdg-open": "bin/xdg-open"}), + self.assertRaises(OpenerUnavailable), + ): + self.reveal() + self.popen.assert_not_called() + + def test_posix_opener_lookup_does_not_depend_on_the_host_path_rules(self): + # Python 3.13 on Windows no longer treats "/usr/bin/x" as absolute. The + # Linux and macOS openers are POSIX paths whatever host runs the code, + # so a host os.path with those rules must not reject them. + windows_313 = SimpleNamespace(isabs=lambda value: False) + with ( + patch.object(file_reveal.os, "path", windows_313), + self.which({"xdg-open": "/usr/bin/xdg-open", "rel": "bin/rel"}), + ): + self.assertEqual(file_reveal._executable("xdg-open"), "/usr/bin/xdg-open") + self.assertIsNone(file_reveal._executable("rel")) + + def test_children_get_a_minimal_environment(self): + inherited = { + "PATH": "/venv/bin:/usr/bin", + "PYTHONPATH": "/comfy", + "LD_LIBRARY_PATH": "/venv/lib", + "VIRTUAL_ENV": "/venv", + "CIVITAI_API_KEY": "secret", + "HOME": "/home/u", + "DISPLAY": ":0", + "WAYLAND_DISPLAY": "wayland-0", + "XDG_RUNTIME_DIR": "/run/user/1000", + "XDG_CURRENT_DESKTOP": "KDE", + "DBUS_SESSION_BUS_ADDRESS": "unix:path=/run/user/1000/bus", + "LANG": "en_US.UTF-8", + # What a desktop opener can need (final review m1): the X11 + # cookie under GDM/SDDM, the session and the user's mimeapps. + "XAUTHORITY": "/run/user/1000/gdm/Xauthority", + "DESKTOP_SESSION": "plasma", + "XDG_SESSION_DESKTOP": "KDE", + "XDG_CONFIG_HOME": "/home/u/.config", + "XDG_DATA_DIRS": "/usr/share", + } + with patch.dict(os.environ, inherited, clear=True): + env = file_reveal._child_env() + self.assertEqual( + env, + { + "PATH": file_reveal.SYSTEM_PATH, + "HOME": "/home/u", + "DISPLAY": ":0", + "WAYLAND_DISPLAY": "wayland-0", + "XDG_RUNTIME_DIR": "/run/user/1000", + "XDG_CURRENT_DESKTOP": "KDE", + "DBUS_SESSION_BUS_ADDRESS": "unix:path=/run/user/1000/bus", + "LANG": "en_US.UTF-8", + "XAUTHORITY": "/run/user/1000/gdm/Xauthority", + "DESKTOP_SESSION": "plasma", + "XDG_SESSION_DESKTOP": "KDE", + "XDG_CONFIG_HOME": "/home/u/.config", + "XDG_DATA_DIRS": "/usr/share", + }, + ) + self.assertNotIn("/venv", file_reveal.SYSTEM_PATH) + + def test_linux_asks_the_file_manager_service_and_waits_for_its_reply(self): + self.popen.return_value.wait.return_value = 0 + with ( + patch.object(file_reveal.sys, "platform", "linux"), + self.which({"dbus-send": "/usr/bin/dbus-send", "xdg-open": "/x"}), + ): + self.reveal() + self.assertEqual( + self.argv(), + [ + "/usr/bin/dbus-send", + "--session", + "--print-reply", + "--reply-timeout=2000", + "--dest=org.freedesktop.FileManager1", + "--type=method_call", + "/org/freedesktop/FileManager1", + "org.freedesktop.FileManager1.ShowItems", + f"array:string:{self.path.as_uri()}", + "string:", + ], + ) + self.assertEqual(self.popen.call_count, 1) + self.popen.return_value.wait.assert_called_once_with( + timeout=file_reveal.DBUS_WAIT + ) + self.assert_detached() + self.reaped.assert_not_called() + + def test_linux_falls_back_to_xdg_open_on_the_folder(self): + both = {"dbus-send": "/usr/bin/dbus-send", "xdg-open": "/usr/bin/xdg-open"} + cases = { + "missing": ({"xdg-open": "/usr/bin/xdg-open"}, None), + "no reply": (both, 1), + "start error": (both, OSError("no bus")), + "hung": (both, subprocess.TimeoutExpired("dbus-send", 3)), + } + for name, (available, outcome) in cases.items(): + with self.subTest(name): + self.popen.reset_mock(side_effect=True, return_value=True) + self.reaped.reset_mock() + dbus = MagicMock() + if isinstance(outcome, OSError): + self.popen.side_effect = [outcome, MagicMock()] + elif isinstance(outcome, subprocess.TimeoutExpired): + dbus.wait.side_effect = [outcome, None] + self.popen.side_effect = [dbus, MagicMock()] + else: + self.popen.return_value.wait.return_value = outcome + with ( + patch.object(file_reveal.sys, "platform", "linux"), + self.which(available), + ): + self.reveal() + self.assertEqual( + self.popen.call_args.args[0], + ["/usr/bin/xdg-open", str(self.path.parent)], + ) + self.assert_detached(self.popen.call_count - 1) + self.reaped.assert_called_once() + if name == "hung": + dbus.kill.assert_called_once_with() + + def test_linux_without_any_opener_is_unavailable(self): + with ( + patch.object(file_reveal.sys, "platform", "linux"), + self.which({}), + self.assertRaises(OpenerUnavailable), + ): + self.reveal() + self.popen.assert_not_called() + + def test_relative_paths_are_refused(self): + with self.assertRaises(ValueError): + reveal_in_file_manager(Path("sub/a.png"), self.root) + self.popen.assert_not_called() + + def test_containment_is_rechecked_just_before_launching(self): + # A parent swapped for a symlink after the route resolved the path + # (security review L4) must not open a folder outside the root. + outside = self.root.parent / "elsewhere" + outside.mkdir() + (outside / "a.png").write_bytes(b"png") + (self.root / "sub" / "a.png").unlink() + (self.root / "sub").rmdir() + try: + os.symlink(outside, self.root / "sub", target_is_directory=True) + except (OSError, NotImplementedError): + self.skipTest("symlinks unavailable") + missing = self.root / "gone.png" + folder = self.root / "folder.png" + folder.mkdir() + for path in (self.path, missing, folder): + for platform in ("linux", "darwin", "win32"): + with ( + self.subTest(path=path.name, platform=platform), + patch.object(file_reveal.sys, "platform", platform), + patch.object(file_reveal, "_windows_libraries") as shell, + self.assertRaises(file_reveal.RevealTargetChanged), + ): + self.reveal(path) + shell.assert_not_called() + self.popen.assert_not_called() + + def test_a_path_on_another_drive_is_not_contained(self): + # Windows: commonpath raises ValueError across drives. + with ( + patch.object(file_reveal.os.path, "commonpath", side_effect=ValueError), + self.assertRaises(file_reveal.RevealTargetChanged), + ): + self.reveal() + self.popen.assert_not_called() + + +class TestReaping(unittest.TestCase): + def test_reap_waits_on_a_daemon_thread(self): + process = MagicMock() + with patch.object(file_reveal.threading, "Thread") as thread: + file_reveal._reap(process) + thread.assert_called_once() + kwargs = thread.call_args.kwargs + self.assertIs(kwargs["target"], process.wait) + self.assertIs(kwargs["daemon"], True) + thread.return_value.start.assert_called_once_with() + + def test_reveal_runs_on_its_own_single_worker(self): + executor = file_reveal.reveal_executor() + self.assertIs(file_reveal.reveal_executor(), executor) + self.assertEqual(executor._max_workers, 1) + name = executor.submit(lambda: file_reveal.threading.current_thread().name) + self.assertTrue(name.result(timeout=5).startswith("pm-reveal")) + + +class FakeShell: + """ole32 and shell32 stand-ins recording the shell calls (H1).""" + + def __init__(self, init=0, pidl=1234, select=0): + self.calls = [] + self.ole32 = types.SimpleNamespace( + CoInitializeEx=self.fn("CoInitializeEx", init), + CoUninitialize=self.fn("CoUninitialize", None), + ) + self.shell32 = types.SimpleNamespace( + ILCreateFromPathW=self.fn("ILCreateFromPathW", pidl), + SHOpenFolderAndSelectItems=self.fn("SHOpenFolderAndSelectItems", select), + ILFree=self.fn("ILFree", None), + ) + + def fn(self, name, result): + def call(*args): + self.calls.append((name, args)) + if isinstance(result, Exception): + raise result + return result + + return MagicMock(side_effect=call) + + def names(self): + return [name for name, _ in self.calls] + + +class TestWindowsReveal(unittest.TestCase): + """The Windows opener with the shell libraries and os.startfile faked. + The folder fallback is recorded in the same call list, so its place + inside the COM bracket is visible (final review m2).""" + + def reveal(self, shell, path=r"C:\out\a,\b.png"): + def startfile(*args): + shell.calls.append(("startfile", args)) + + with ( + patch.object( + file_reveal, + "_windows_libraries", + return_value=(shell.ole32, shell.shell32), + ), + patch.object( + file_reveal.os, + "startfile", + MagicMock(side_effect=startfile), + create=True, + ), + ): + file_reveal._reveal_windows(Path(path)) + return shell + + def test_selects_the_item_and_frees_everything(self): + path = r"C:\out\a,\Windows\System32\calc.exe,b\x.png" + shell = self.reveal(FakeShell(), path) + self.assertEqual( + shell.names(), + [ + "CoInitializeEx", + "ILCreateFromPathW", + "SHOpenFolderAndSelectItems", + "ILFree", + "CoUninitialize", + ], + ) + self.assertEqual(dict(shell.calls)["ILCreateFromPathW"], (str(Path(path)),)) + self.assertEqual( + dict(shell.calls)["SHOpenFolderAndSelectItems"], (1234, 0, None, 0) + ) + self.assertEqual(dict(shell.calls)["ILFree"], (1234,)) + + def test_declares_the_shell_signatures(self): + shell = self.reveal(FakeShell()) + create = shell.shell32.ILCreateFromPathW + self.assertEqual(create.argtypes, [ctypes.c_wchar_p]) + self.assertIs(create.restype, ctypes.c_void_p) + open_select = shell.shell32.SHOpenFolderAndSelectItems + self.assertEqual( + open_select.argtypes, + [ctypes.c_void_p, ctypes.c_uint, ctypes.c_void_p, ctypes.c_ulong], + ) + self.assertIs(open_select.restype, ctypes.c_long) + self.assertEqual(shell.shell32.ILFree.argtypes, [ctypes.c_void_p]) + self.assertEqual( + shell.ole32.CoInitializeEx.argtypes, [ctypes.c_void_p, ctypes.c_ulong] + ) + self.assertIs(shell.ole32.CoInitializeEx.restype, ctypes.c_long) + + def test_already_initialized_com_is_used_and_left_alone(self): + shell = self.reveal(FakeShell(init=file_reveal.RPC_E_CHANGED_MODE)) + self.assertIn("SHOpenFolderAndSelectItems", shell.names()) + self.assertNotIn("CoUninitialize", shell.names()) + self.assertNotIn("startfile", shell.names()) + shell = self.reveal(FakeShell(init=file_reveal.S_FALSE)) + self.assertNotIn("startfile", shell.names()) + self.assertEqual(shell.names()[-1], "CoUninitialize") + + def test_the_folder_fallback_runs_inside_com(self): + # ShellExecute callers should initialize COM first (final review m2). + for name, shell in { + "no pidl": FakeShell(pidl=None), + "select failed": FakeShell(select=-2147467259), + "raises": FakeShell(select=OSError("access violation")), + }.items(): + with self.subTest(name): + names = self.reveal(shell).names() + self.assertEqual(names[0], "CoInitializeEx") + self.assertEqual(names[-2:], ["startfile", "CoUninitialize"]) + if name != "no pidl": + self.assertIn("ILFree", names) + + def test_failed_com_still_opens_the_folder_without_balancing(self): + for init in (-2147024882, OSError("no ole32 export")): + with self.subTest(init=init): + names = self.reveal(FakeShell(init=init)).names() + self.assertEqual(names, ["CoInitializeEx", "startfile"]) + + def test_missing_libraries_open_the_folder(self): + startfile = MagicMock() + with ( + patch.object(file_reveal, "_windows_libraries", side_effect=OSError), + patch.object(file_reveal.os, "startfile", startfile, create=True), + ): + file_reveal._reveal_windows(Path(r"C:\out\a.png")) + startfile.assert_called_once_with(str(Path(r"C:\out\a.png").parent), "explore") + + def test_libraries_are_unavailable_off_windows(self): + if sys.platform == "win32": + self.skipTest("loads the real shell libraries on Windows") + with self.assertRaises((AttributeError, OSError)): + file_reveal._windows_libraries() + + +@unittest.skipUnless(sys.platform == "win32", "the real Windows shell libraries") +class TestRealWindowsShell(unittest.TestCase): + """Final review I1: the declared signatures against the real ole32 and + shell32, without SHOpenFolderAndSelectItems (no window opens on CI).""" + + def setUp(self): + self.ole32, self.shell32 = file_reveal._windows_libraries() + file_reveal._declare_shell_signatures(self.ole32, self.shell32) + + def test_a_pidl_is_created_for_a_real_file_and_freed(self): + with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as folder: + path = Path(folder) / "a,b.png" + path.write_bytes(b"png") # write_bytes closes its handle + pidl = self.shell32.ILCreateFromPathW(str(path)) + self.assertIsInstance(pidl, int) + self.assertNotEqual(pidl, 0) + self.shell32.ILFree(pidl) + self.assertIsNone(self.shell32.ILCreateFromPathW(str(path) + ".gone")) + path.unlink() + + def probe(self): + """CoInitializeEx's answer on this thread, balanced at once.""" + result = self.ole32.CoInitializeEx( + None, + file_reveal.COINIT_APARTMENTTHREADED | file_reveal.COINIT_DISABLE_OLE1DDE, + ) + if result in (file_reveal.S_OK, file_reveal.S_FALSE): + self.ole32.CoUninitialize() + return result + + def test_com_is_entered_and_balanced_through_the_module_helper(self): + before = self.probe() + with file_reveal._com_apartment(self.ole32) as ready: + self.assertIs(ready, True) + # An unbalanced bracket would leave COM initialized: S_FALSE after S_OK. + self.assertEqual(self.probe(), before) + + +class TestRevealRoute(ImageAPITestCase): + async def get_application(self): + app = await super().get_application() + self.opened, self.roots, self.threads = [], [], [] + self.outcome = None + + def opener(path, root): + self.threads.append(file_reveal.threading.current_thread().name) + if self.outcome is not None: + raise self.outcome + self.opened.append(path) + self.roots.append(root) + + self.api._reveal_opener = opener + self.api._reveal_gate = file_reveal.RevealGate(cooldown=0) + return app + + def origin(self): + return str(self.client.make_url("/").origin()) + + def scope(self, index=0): + roots = self.api._get_all_output_dirs() + return self.api._gallery_root_scope(roots[index], index) + + async def post(self, body, **request_headers): + # Like the workspace's fetch: a same-origin JSON POST. + sent = {"Origin": self.origin(), **request_headers} + sent = {key: value for key, value in sent.items() if value is not None} + response = await self.client.post(REVEAL, json=body, headers=sent) + return response.status, await response.json() + + def body(self, path="sub/a.png", **changes): + return {"root_index": 0, "root_scope": self.scope(), "path": path, **changes} + + async def test_loopback_request_opens_the_resolved_file(self): + target = make_png(self.output_dir / "sub" / "a.png") + status, data = await self.post(self.body()) + self.assertEqual((status, data), (200, {"success": True})) + self.assertEqual(self.opened, [Path(os.path.realpath(target))]) + self.assertEqual(self.roots, [self.output_dir]) + # Its own worker, never ComfyUI's shared default executor (L1). + self.assertTrue(self.threads[0].startswith("pm-reveal"), self.threads) + + async def test_target_changed_before_launch_is_404(self): + make_png(self.output_dir / "sub" / "a.png") + self.outcome = file_reveal.RevealTargetChanged() + status, data = await self.post(self.body()) + self.assertEqual(status, 404) + self.assertEqual(data, {"success": False, "error": "Image file not found"}) + + async def test_non_loopback_remote_is_not_local(self): + make_png(self.output_dir / "sub" / "a.png") + with patch.object( + web.BaseRequest, "remote", new_callable=PropertyMock + ) as remote: + remote.return_value = "192.168.1.20" + status, data = await self.post(self.body()) + self.assertEqual(status, 403) + self.assertEqual(data["code"], "not-local") + self.assertIs(data["success"], False) + self.assertEqual(self.opened, []) + + async def test_loopback_with_a_proxy_header_is_not_local(self): + make_png(self.output_dir / "sub" / "a.png") + for name in PROXY_HEADERS: + with self.subTest(header=name): + status, data = await self.post(self.body(), **{name: "127.0.0.1"}) + self.assertEqual((status, data["code"]), (403, "not-local")) + status, data = await self.post(self.body(), Host="evil.example:8188") + self.assertEqual((status, data["code"]), (403, "not-local")) + self.assertEqual(self.opened, []) + + async def test_missing_or_foreign_origin_is_not_local(self): + make_png(self.output_dir / "sub" / "a.png") + port = self.client.make_url("/").port + for origin in (None, f"http://127.0.0.1:{port + 1}", "null"): + with self.subTest(origin=origin): + status, data = await self.post(self.body(), Origin=origin) + self.assertEqual((status, data["code"]), (403, "not-local")) + status, data = await self.post(self.body(), **{"Sec-Fetch-Site": "same-site"}) + self.assertEqual((status, data["code"]), (403, "not-local")) + status, _ = await self.post(self.body(), **{"Sec-Fetch-Site": "same-origin"}) + self.assertEqual(status, 200) + self.assertEqual(len(self.opened), 1) + + async def test_only_json_bodies_are_accepted(self): + # text/plain would be a CORS "simple" request with no preflight (L2). + make_png(self.output_dir / "sub" / "a.png") + body = json.dumps(self.body()) + for content_type in ("text/plain", "application/x-www-form-urlencoded", None): + with self.subTest(content_type=content_type): + sent = {"Origin": self.origin()} + if content_type: + sent["Content-Type"] = content_type + response = await self.client.post( + REVEAL, data=body.encode(), headers=sent + ) + self.assertEqual(response.status, 415) + data = await response.json() + self.assertEqual( + (data["success"], data["code"]), (False, "unsupported-media-type") + ) + self.assertEqual(self.opened, []) + + async def test_a_reveal_in_progress_or_cooling_down_is_busy(self): + make_png(self.output_dir / "sub" / "a.png") + gate = self.api._reveal_gate + self.assertTrue(gate.try_enter()) + status, data = await self.post(self.body()) + self.assertEqual(status, 429) + self.assertEqual(data, {**data, "success": False, "code": "busy"}) + gate.leave() + now = [50.0] + self.api._reveal_gate = file_reveal.RevealGate( + cooldown=1.0, clock=lambda: now[0] + ) + statuses = [(await self.post(self.body()))[0] for _ in range(2)] + now[0] += 1.0 + statuses.append((await self.post(self.body()))[0]) + self.assertEqual(statuses, [200, 429, 200]) + self.assertEqual(len(self.opened), 2) + + async def test_a_failed_reveal_releases_the_gate(self): + make_png(self.output_dir / "sub" / "a.png") + self.outcome = OSError("cannot start") + status, _ = await self.post(self.body()) + self.assertEqual(status, 500) + self.outcome = None + status, _ = await self.post(self.body()) + self.assertEqual(status, 200) + + async def test_default_gate_is_one_per_process_with_a_cooldown(self): + gate = type(self.api)._reveal_gate + self.assertIsInstance(gate, file_reveal.RevealGate) + self.assertEqual(gate.cooldown, file_reveal.REVEAL_COOLDOWN) + self.assertGreaterEqual(file_reveal.REVEAL_COOLDOWN, 1.0) + + async def test_resolution_errors_are_404_and_never_log_the_path(self): + # ENAMETOOLONG and friends must not escape as a non-JSON 500 (L3). + make_png(self.output_dir / "sub" / "a.png") + secret = "sub/" + "n" * 300 + "/a.png" + with self.assertLogs(self.api.logger, level="DEBUG") as logs: + self.api.logger.debug("start") + status, data = await self.post(self.body(path=secret)) + self.assertEqual(status, 404) + self.assertEqual(data, {"success": False, "error": "Image file not found"}) + with patch.object( + type(self.api), + "_reveal_path", + staticmethod(MagicMock(side_effect=OSError(36, "too long", secret))), + ): + status, data = await self.post(self.body(path=secret)) + self.assertEqual(status, 404) + self.assertIs(data["success"], False) + for record in logs.records: + if record.levelno >= logging.WARNING: + self.assertNotIn("n" * 300, record.getMessage()) + self.assertEqual(self.opened, []) + + async def test_start_failure_never_logs_the_path_at_error_level(self): + make_png(self.output_dir / "sub" / "a.png") + self.outcome = OSError(2, "No such file", str(self.output_dir / "sub")) + with self.assertLogs(self.api.logger, level="DEBUG") as logs: + status, _ = await self.post(self.body()) + self.assertEqual(status, 500) + errors = [r for r in logs.records if r.levelno >= logging.ERROR] + self.assertTrue(errors) + for record in errors: + self.assertNotIn(str(self.output_dir), record.getMessage()) + self.assertIsNone(record.exc_info) + + async def test_changed_or_missing_scope_is_rejected(self): + make_png(self.output_dir / "sub" / "a.png") + status, data = await self.post(self.body(root_scope="0" * 64)) + self.assertEqual((status, data["code"]), (409, "source-changed")) + status, data = await self.post(self.body(root_index=3)) + self.assertEqual((status, data["code"]), (409, "source-changed")) + for scope in (None, "ABC", 7, "g" * 64): + with self.subTest(scope=scope): + status, _ = await self.post(self.body(root_scope=scope)) + self.assertEqual(status, 400) + body = self.body() + del body["root_scope"] + status, _ = await self.post(body) + self.assertEqual(status, 400) + self.assertEqual(self.opened, []) + + async def test_body_types_are_strict(self): + make_png(self.output_dir / "sub" / "a.png") + for changes in ( + {"root_index": True}, + {"root_index": "0"}, + {"root_index": -1}, + {"root_index": 0.0}, + {"path": 5}, + {"path": ""}, + {"path": "a" * 5000 + ".png"}, + {"path": "sub/a\0.png"}, + ): + with self.subTest(changes=changes): + status, data = await self.post(self.body(**changes)) + self.assertEqual(status, 400, data) + response = await self.client.post( + REVEAL, + data="[1]", + headers={"Origin": self.origin(), "Content-Type": "application/json"}, + ) + self.assertEqual(response.status, 400) + self.assertEqual(self.opened, []) + + async def test_traversal_and_absolute_paths_are_rejected(self): + make_png(self.output_dir / "sub" / "a.png") + outside = make_png(self.comfy_dir / "outside.png") + for path in ( + "../outside.png", + "sub/../../outside.png", + "sub\\..\\..\\outside.png", + str(outside), + "/" + outside.name, + "C:/outside.png", + "C:outside.png", + ): + with self.subTest(path=path): + status, data = await self.post(self.body(path=path)) + self.assertEqual(status, 400, data) + self.assertEqual(self.opened, []) + + async def test_symlink_out_of_the_root_is_refused(self): + outside = make_png(self.comfy_dir / "outside.png") + self._make_symlink(outside, self.output_dir / "link.png") + status, _ = await self.post(self.body(path="link.png")) + self.assertIn(status, (403, 404)) + self.assertEqual(self.opened, []) + + async def test_missing_non_media_and_non_regular_files(self): + (self.output_dir / "folder.png").mkdir() + (self.output_dir / "notes.txt").write_text("x") + for path, expected in ( + ("sub/missing.png", 404), + ("folder.png", 404), + ("notes.txt", 403), + ): + with self.subTest(path=path): + status, data = await self.post(self.body(path=path)) + self.assertEqual(status, expected, data) + self.assertIs(data["success"], False) + self.assertEqual(self.opened, []) + + async def test_unavailable_opener_is_501_and_start_failure_500(self): + make_png(self.output_dir / "sub" / "a.png") + self.outcome = OpenerUnavailable() + status, data = await self.post(self.body()) + self.assertEqual(status, 501) + self.assertEqual(data, {**data, "success": False, "code": "unavailable"}) + self.outcome = OSError("cannot start") + status, data = await self.post(self.body()) + self.assertEqual(status, 500) + self.assertIs(data["success"], False) + self.assertNotIn("cannot start", data["error"]) + + async def test_default_opener_is_the_platform_file_manager(self): + self.assertIs(type(self.api)._reveal_opener, file_reveal.reveal_in_file_manager) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/file_reveal.py b/utils/file_reveal.py new file mode 100644 index 0000000..61f5682 --- /dev/null +++ b/utils/file_reveal.py @@ -0,0 +1,451 @@ +"""Show one gallery file in this computer's file manager (TESTING.md D38). + +The route that calls this is local-only (``is_local_request``) and +single-flight (``RevealGate``), and hands in a path it has already resolved +inside a gallery root, which is re-checked just before launching. Windows +goes through the shell API (no command line exists to parse); macOS and Linux +openers run as an argument list with an absolute executable from a fixed +system PATH, no shell, no inherited streams and a minimal environment, so +neither a file name nor ``PATH`` can change what runs or what it parses. +""" + +import ipaddress +import os +import posixpath +import shutil + +# Only POSIX argv launches of a fixed opener (see _launch); Windows never +# starts a process (see _select_item). +import subprocess # nosec B404 +import sys +import threading +import time +from collections.abc import Mapping +from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager +from pathlib import Path +from urllib.parse import urlsplit + +# A local reverse proxy makes every client look like loopback, so a request +# that carries any forwarding-style header counts as remote: these names, and +# every X-Forwarded-*, X-Original-* and *-Client-IP header. +FORWARDING_HEADERS = frozenset( + ( + "forwarded", + "via", + "x-real-ip", + "cf-connecting-ip", + "true-client-ip", + "x-client-ip", + "x-cluster-client-ip", + "fastly-client-ip", + ) +) +FORWARDING_PREFIXES = ("x-forwarded-", "x-original-") +FORWARDING_SUFFIXES = ("-client-ip",) +DEFAULT_PORTS = {"http": 80, "https": 443} +# One reveal at a time per process, then a pause before the next. +REVEAL_COOLDOWN = 1.0 +# dbus-send waits this long for the FileManager1 reply, so its exit status +# says whether a file manager really took the request. +DBUS_REPLY_TIMEOUT_MS = 2000 +# The dbus-send process itself gets a little longer before it is killed. +DBUS_WAIT = 3.0 +FILE_MANAGER_DEST = "org.freedesktop.FileManager1" +# Openers are looked up here only, never on ComfyUI's (venv) PATH, and the +# children get this as their PATH. +SYSTEM_PATH = os.pathsep.join( + ("/usr/local/bin", "/usr/bin", "/bin", "/run/current-system/sw/bin") +) +MACOS_OPEN = "/usr/bin/open" +# What a desktop opener needs from ComfyUI's environment; everything else +# (venv PATH, PYTHONPATH, LD_LIBRARY_PATH, API keys) stays behind. +CHILD_ENV_NAMES = ( + "HOME", + "DISPLAY", + "WAYLAND_DISPLAY", + "XDG_RUNTIME_DIR", + "XDG_CURRENT_DESKTOP", + "XDG_SESSION_TYPE", + "XDG_SESSION_DESKTOP", + "DESKTOP_SESSION", + "XDG_DATA_DIRS", + "XDG_CONFIG_DIRS", + "XDG_CONFIG_HOME", + # The X11 cookie: a non-default path under GDM or SDDM. + "XAUTHORITY", + "DBUS_SESSION_BUS_ADDRESS", + "LANG", + "LC_ALL", +) + + +class OpenerUnavailable(Exception): + """No file manager can be started on this computer.""" + + +class RevealTargetChanged(Exception): + """The file is no longer a regular file inside its root.""" + + +_executor_lock = threading.Lock() +_executor: ThreadPoolExecutor | None = None + + +def reveal_executor() -> ThreadPoolExecutor: + """The one worker that runs openers, so a slow file manager or dbus reply + never holds a thread of ComfyUI's shared default executor.""" + global _executor + with _executor_lock: + if _executor is None: + _executor = ThreadPoolExecutor( + max_workers=1, thread_name_prefix="pm-reveal" + ) + return _executor + + +def _loopback_address(value: str | None) -> bool: + if not value: + return False + try: + address = ipaddress.ip_address(value) + except ValueError: + return False + mapped = getattr(address, "ipv4_mapped", None) + return (mapped or address).is_loopback + + +def _loopback_host(host: str) -> bool: + """True for a Host name naming this computer. + + Rejects every other name, so a DNS-rebinding page whose name now resolves + to 127.0.0.1 still reads as remote. + """ + host = host.rstrip(".").lower() + if host == "localhost" or host.endswith(".localhost"): + return True + return _loopback_address(host.strip("[]")) + + +def _authority(authority: str) -> tuple[str, int | None] | None: + """``(host, port)`` of a Host value, or None when it does not parse.""" + try: + parts = urlsplit(f"//{authority}") + host, port = parts.hostname, parts.port + except ValueError: + return None + return (host, port) if host else None + + +def _forwarded(headers: Mapping) -> bool: + for name in headers.keys(): + name = name.lower() + if ( + name in FORWARDING_HEADERS + or name.startswith(FORWARDING_PREFIXES) + or name.endswith(FORWARDING_SUFFIXES) + ): + return True + return False + + +def _same_origin(origin: str, scheme: str, host: tuple[str, int | None]) -> bool: + """True when *origin* is exactly this request's scheme://host:port.""" + default = DEFAULT_PORTS.get(scheme) + if default is None: + return False + try: + parts = urlsplit(origin) + origin_host, origin_port = parts.hostname, parts.port + except ValueError: + return False + if ( + parts.scheme != scheme + or not origin_host + or parts.username is not None + or parts.password is not None + or parts.path + or parts.query + or parts.fragment + ): + return False + host_name, host_port = host + return (origin_host.rstrip("."), origin_port or default) == ( + host_name.rstrip("."), + host_port or default, + ) + + +def is_local_request(remote: str | None, headers: Mapping, scheme="http") -> bool: + """True only for a same-origin browser request from this computer. + + That is: no forwarding-style header, a loopback client, a loopback name + in Host, an ``Origin`` that is exactly this request's scheme://host:port + (browsers send it on every fetch POST, so a missing one is refused), and + ``Sec-Fetch-Site: same-origin`` whenever that header is sent. + """ + if _forwarded(headers): + return False + if not _loopback_address(remote): + return False + host = _authority(headers.get("Host", "")) + if host is None or not _loopback_host(host[0]): + return False + origin = headers.get("Origin") + if not origin or not _same_origin(origin, scheme, host): + return False + site = headers.get("Sec-Fetch-Site") + return site is None or site == "same-origin" + + +class RevealGate: + """Single flight plus a cooldown: at most one reveal runs per process, + and the next may start only *cooldown* seconds after it finished.""" + + def __init__(self, cooldown: float = REVEAL_COOLDOWN, clock=time.monotonic): + self.cooldown = cooldown + self._clock = clock + self._lock = threading.Lock() + self._busy = False + self._finished: float | None = None + + def try_enter(self) -> bool: + with self._lock: + if self._busy: + return False + if ( + self._finished is not None + and self._clock() - self._finished < self.cooldown + ): + return False + self._busy = True + return True + + def leave(self) -> None: + with self._lock: + self._busy = False + self._finished = self._clock() + + +def _executable(name: str) -> str | None: + """Absolute path of *name* on the fixed SYSTEM_PATH, never a relative hit. + + Only the Linux and macOS openers are looked up here, so absoluteness follows + POSIX rules rather than the host's (Python 3.13 on Windows no longer counts + "/usr/bin/x" as absolute). + """ + found = shutil.which(name, path=SYSTEM_PATH) + return found if found and posixpath.isabs(found) else None + + +def _macos_open() -> str | None: + """macOS's own ``open``, at its fixed system path.""" + if os.path.isfile(MACOS_OPEN) and os.access(MACOS_OPEN, os.X_OK): + return MACOS_OPEN + return None + + +def _child_env() -> dict[str, str]: + env = {name: os.environ[name] for name in CHILD_ENV_NAMES if name in os.environ} + return {"PATH": SYSTEM_PATH, **env} + + +def _reap(process: subprocess.Popen) -> None: + """Wait for *process* on a daemon thread, so it never lingers as a zombie + and is never waited on by the caller.""" + threading.Thread(target=process.wait, name="pm-reveal-reaper", daemon=True).start() + + +def _contained_file(path: Path, root: Path) -> Path: + """*path*'s real path when it is still a regular file inside *root*. + + Run right before launching: a parent swapped for a symlink since the + route resolved the path must not open a folder outside the root. + """ + real = os.path.realpath(path) + real_root = os.path.realpath(root) + try: + inside = os.path.commonpath( + [os.path.normcase(real), os.path.normcase(real_root)] + ) == os.path.normcase(real_root) + except ValueError: + inside = False + if not inside or real == real_root or not os.path.isfile(real): + raise RevealTargetChanged() + return Path(real) + + +# HRESULTs as the signed 32-bit values ctypes.c_long returns. +S_OK = 0 +S_FALSE = 1 +RPC_E_CHANGED_MODE = -2147417850 # 0x80010106: COM already set up otherwise +COINIT_APARTMENTTHREADED = 0x2 +COINIT_DISABLE_OLE1DDE = 0x4 + + +def _windows_libraries(): + """Private ole32 and shell32 handles, so the signatures set here never + change another library's view of ``ctypes.windll``.""" + import ctypes + + return ctypes.WinDLL("ole32"), ctypes.WinDLL("shell32") + + +def _declare_shell_signatures(ole32, shell32) -> None: + import ctypes + + ole32.CoInitializeEx.argtypes = [ctypes.c_void_p, ctypes.c_ulong] + ole32.CoInitializeEx.restype = ctypes.c_long + ole32.CoUninitialize.argtypes = [] + ole32.CoUninitialize.restype = None + shell32.ILCreateFromPathW.argtypes = [ctypes.c_wchar_p] + shell32.ILCreateFromPathW.restype = ctypes.c_void_p + shell32.SHOpenFolderAndSelectItems.argtypes = [ + ctypes.c_void_p, + ctypes.c_uint, + ctypes.c_void_p, + ctypes.c_ulong, + ] + shell32.SHOpenFolderAndSelectItems.restype = ctypes.c_long + shell32.ILFree.argtypes = [ctypes.c_void_p] + shell32.ILFree.restype = None + + +def _shell_libraries(): + """The declared ole32 and shell32, or None where they cannot be loaded.""" + try: + ole32, shell32 = _windows_libraries() + _declare_shell_signatures(ole32, shell32) + except Exception: # no shell libraries: the folder fallback still runs + return None + return ole32, shell32 + + +@contextmanager +def _com_apartment(ole32): + """The calling thread inside COM for the block; yields whether COM is + usable. Only a successful CoInitializeEx (S_OK or S_FALSE) is balanced, + so the persistent reveal thread never drifts.""" + try: + result = ole32.CoInitializeEx( + None, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE + ) + except Exception: # an unusable ole32 leaves COM alone + result = None + try: + yield result in (S_OK, S_FALSE, RPC_E_CHANGED_MODE) + finally: + if result in (S_OK, S_FALSE): + ole32.CoUninitialize() + + +def _select_item(shell32, path: Path) -> bool: + """Open Explorer on *path*'s folder with *path* selected, via the shell + API. The path is one wide-string value, so no command line is ever + parsed (Explorer splits its own on commas). False on any failure; never + raises into the route.""" + try: + pidl = shell32.ILCreateFromPathW(str(path)) + if not pidl: + return False + try: + return shell32.SHOpenFolderAndSelectItems(pidl, 0, None, 0) >= 0 + finally: + shell32.ILFree(pidl) + except Exception: # any shell failure falls back to the folder + return False + + +def _explore(folder: Path) -> None: + startfile = getattr(os, "startfile", None) + if startfile is None: + raise OpenerUnavailable() + # ShellExecute on the folder itself: no command line is built or parsed. + startfile(str(folder), "explore") + + +def _reveal_windows(path: Path) -> None: + libraries = _shell_libraries() + if libraries is None: + _explore(path.parent) + return + ole32, shell32 = libraries + with _com_apartment(ole32) as ready: + if ready and _select_item(shell32, path): + return + # ShellExecute inside the same apartment: shell extensions may need COM. + _explore(path.parent) + + +def _launch(argv: list[str]) -> subprocess.Popen: + options = { + "stdin": subprocess.DEVNULL, + "stdout": subprocess.DEVNULL, + "stderr": subprocess.DEVNULL, + "close_fds": True, + # Its own session, so stopping ComfyUI does not take the window along. + "start_new_session": True, + "env": _child_env(), + } + # POSIX only (Windows uses the shell API): argv[0] is an absolute opener, + # the rest are fixed flags and one absolute path inside a gallery root, + # passed to execve as separate strings with no shell to re-parse them. + return subprocess.Popen(argv, **options) # nosec B603 + + +def _show_items(path: Path) -> bool: + """Ask the freedesktop FileManager1 service to select *path*; True only + when the service replied (``--print-reply`` makes the exit status real).""" + dbus = _executable("dbus-send") + if dbus is None: + return False + argv = [ + dbus, + "--session", + "--print-reply", + f"--reply-timeout={DBUS_REPLY_TIMEOUT_MS}", + f"--dest={FILE_MANAGER_DEST}", + "--type=method_call", + "/org/freedesktop/FileManager1", + f"{FILE_MANAGER_DEST}.ShowItems", + f"array:string:{path.as_uri()}", + "string:", + ] + try: + process = _launch(argv) + except OSError: + return False + try: + return process.wait(timeout=DBUS_WAIT) == 0 + except subprocess.TimeoutExpired: + process.kill() + process.wait() + return False + + +def reveal_in_file_manager(path: Path, root: Path) -> None: + """Open the file manager at *path*, a regular file inside *root*. + + Re-checks that containment first (RevealTargetChanged). Raises + OpenerUnavailable when no opener exists and OSError when the one found + cannot start. Never waits on the file manager itself. + """ + path = Path(path) + if not path.is_absolute(): + raise ValueError("path must be absolute") + path = _contained_file(path, Path(root)) + if sys.platform == "win32": + _reveal_windows(path) + return + if sys.platform == "darwin": + opener = _macos_open() + if opener is None: + raise OpenerUnavailable() + _reap(_launch([opener, "-R", str(path)])) + return + if _show_items(path): + return + opener = _executable("xdg-open") + if opener is None: + raise OpenerUnavailable() + _reap(_launch([opener, str(path.parent)])) diff --git a/web/js/workspace/file-inspector.mjs b/web/js/workspace/file-inspector.mjs index 3a906eb..7e694c2 100644 --- a/web/js/workspace/file-inspector.mjs +++ b/web/js/workspace/file-inspector.mjs @@ -4,6 +4,7 @@ import { createInspectionController } from './inspection-state.mjs'; import { createSendButton } from './prompt-sender.mjs'; import { createWorkspaceIcon } from './icons.mjs'; import { navigateTo } from './state.mjs'; +import { showWorkspaceToast } from './toast.mjs'; const RAW_KEYS = ['prompt', 'workflow', 'parameters', 'comfyassets_prompt_pairs']; const RAW_LIMIT = 'Raw metadata exceeds the response limit. Download the original file to inspect it externally.'; @@ -16,6 +17,13 @@ const REPRESENTATIONS = { limited: 'Record exceeds inspection limits; text is unavailable', }; const TEXTLESS = new Set(['invalid', 'limited']); +const REVEAL_URL = '/prompt_manager/images/reveal'; +// D38 Show in folder: the route's answers, in the maintainer's words. +const REVEAL_OPENED = 'Opened the folder on this computer.'; +const REVEAL_NOT_LOCAL = 'Show in folder works only in a browser on the computer running ComfyUI.'; +const REVEAL_UNAVAILABLE = 'No file manager is available on this computer.'; +const REVEAL_BUSY = 'The folder is already opening.'; +const REVEAL_FAILED = 'The folder could not be opened. Try again.'; const FACTS = [['seed', 'Seed'], ['steps', 'Steps'], ['cfg', 'CFG'], ['sampler', 'Sampler'], ['scheduler', 'Scheduler'], ['model', 'Model']]; function safeFilename(value) { @@ -63,6 +71,23 @@ export function inspectionDownloads(record) { return result; } +/** The reveal body for a listed file's identity (`root_index`, `root_scope`, `path`), else null (D38). */ +export function revealTarget(identity) { + if (identity?.kind !== 'rooted-file') return null; + const {root_index, root_scope, path} = identity; + if (!Number.isInteger(root_index) || root_index < 0 || typeof root_scope !== 'string' || !/^[0-9a-f]{64}$/.test(root_scope) || + typeof path !== 'string' || !path) return null; + return {root_index, root_scope, path}; +} +/** The toast for a reveal outcome: null is success, otherwise the request's error (D38). */ +export function revealOutcome(error) { + if (error === null) return REVEAL_OPENED; + if (error?.status === 403 && error.code === 'not-local') return REVEAL_NOT_LOCAL; + if (error?.status === 501) return REVEAL_UNAVAILABLE; + if (error?.status === 429 && error.code === 'busy') return REVEAL_BUSY; + return REVEAL_FAILED; +} + function el(tag, text, className) { const node = document.createElement(tag); if (text !== undefined) node.textContent = text; @@ -82,7 +107,8 @@ function disclosure(label) { function definitionList(entries, className) { const list = el('dl', undefined, className); for (const [label, value] of entries) { - const row = el('div'); row.append(el('dt', label), el('dd', value ?? 'Not available')); list.append(row); + // A value is its text, or a prepared `dd` (one a caller keeps to add to, such as D38's Path). + const row = el('div'); row.append(el('dt', label), typeof value === 'object' && value !== null ? value : el('dd', value ?? 'Not available')); list.append(row); } return list; } @@ -130,7 +156,7 @@ function prefixedChip(prefix, text) { * `variant: 'full'` (the viewer's `#viewer-details`, imageDetails(i, true)) leaves out the cover and adds "File & * provenance" and "Workflow & raw metadata" with the downloads. `inline: true` means `panel`; neither means `full`. */ -export function mountFileInspector(container, {sender, parentScope, variant, inline = false, promptContext = '', onTagFilter, onRetry, onRefresh, onInteraction = () => {}}) { +export function mountFileInspector(container, {api, sender, parentScope, variant, inline = false, promptContext = '', onTagFilter, onRetry, onRefresh, onInteraction = () => {}}) { if (parentScope.signal.aborted) return {setState() {}, dispose() {}}; const scope = childScope(parentScope); let contentScope; @@ -183,7 +209,7 @@ export function mountFileInspector(container, {sender, parentScope, variant, inl if (linked && library.category) badges.append(prefixedChip(library.metadata_scope === 'pair' ? 'Recipe category: ' : 'Positive prompt category: ', library.category)); if (badges.childNodes.length) inner.append(badges); const prompts = el('div'); inner.append(prompts); - const blocks = mountPromptBlocks(prompts, {sender, parentScope:contentScope, includePairAction:false, contextLabel:promptContext}); + const blocks = mountPromptBlocks(prompts, {sender, parentScope:contentScope, includePairAction:false, contextLabel:promptContext, title}); blocks.setState({prompts:record.prompts}); contentScope.cleanup(blocks.dispose); if (linked && library.tags.length) { // D21: a chip sets the Images Tags filter, which matches the linked positive's tags; a recipe's own tags are @@ -236,13 +262,19 @@ export function mountFileInspector(container, {sender, parentScope, variant, inl const {file, library} = record, section = disclosure('File & provenance'); const reported = record.identity.kind === 'reported-location'; section.append(el('p', reported ? 'Current file at the reported location · identity unverified' : 'Current file at this source; historical bytes are not verified.', 'small')); - if (file) section.append(definitionList([ - ['Source', file.source_label], ['Path', record.identity.path], ['Size', formatBytes(file.size)], + const path = el('dd', record.identity.path ?? 'Not available'); + const facts = file && definitionList([ + ['Source', file.source_label], ['Path', path], ['Size', formatBytes(file.size)], ['Dimensions', file.width && file.height ? `${file.width} × ${file.height}` : null], ['Format', file.format || `${extensionOf(record)} extension`], ['Media type', file.media_type], ['Modified', file.modified_time !== null ? new Date(file.modified_time * 1000).toLocaleString() : null], - ], 'meta-grid')); - else section.append(el('p', 'Current file observations are unavailable for this inspection response.', 'small')); + ], 'meta-grid'); + if (facts) { + // D38: "Show in folder" follows the Path value. + const target = api ? revealTarget(record.identity) : null; + if (target) path.append(' ', revealButton(target)); + section.append(facts); + } else section.append(el('p', 'Current file observations are unavailable for this inspection response.', 'small')); // The concept's "Prompt evidence: execution recorded" line, in the D21 badge's words; no badge, no line. if (badge) section.append(el('p', `Prompt evidence: ${badge.toLowerCase()}`, 'small')); if (reported) { @@ -258,6 +290,25 @@ export function mountFileInspector(container, {sender, parentScope, variant, inl } return section; } + /** D38: "Show in folder" right after the Path value, for a listed file only; the server opens its file manager. */ + function revealButton(target) { + const button = createButton('Show in folder'); + button.setAttribute('aria-label', `Show ${target.path.split('/').at(-1)} in folder`); + let pending = false; + contentScope.listen(button, 'click', async () => { + if (pending) return; + pending = true; button.setAttribute('aria-busy', 'true'); + let error = null; + try { + await api.post(REVEAL_URL, target, {signal:contentScope.signal}); + } catch (failure) { + error = failure ?? {}; + } + pending = false; button.removeAttribute('aria-busy'); + if (!contentScope.signal.aborted) showWorkspaceToast(revealOutcome(error)); + }); + return button; + } /** "Workflow & raw metadata": each supported record (`pre[tabindex=0] > code`), then the two downloads. */ function rawMetadata(record) { const {metadata} = record, section = disclosure('Workflow & raw metadata'); diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index 4460a34..c17697a 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -232,7 +232,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage checked:visit?.checked, onCategorySet:() => { loadCategories(); reset(); }}); if (bar) main.append(bar.element); main.append(body); - const mediaViewer = createMediaViewer({container:main,parentScope:scope,inspectionController:inspection,sender, + 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 => { const index = state?.items.findIndex(entry => imageKey(entry) === imageKey(image)); @@ -649,15 +649,26 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage if (scope.signal.aborted) return; renderLoadStatus(); if (found) reset(); - if (!scope.signal.aborted) showWorkspaceToast(rescanFinished(result)); + if (!scope.signal.aborted) announceRescan(rescanFinished(result)); } catch { // A failed rescan leaves the collection and any discovery alert as they were. - if (!scope.signal.aborted) showWorkspaceToast(RESCAN_FAILED); + if (!scope.signal.aborted) announceRescan(RESCAN_FAILED); } finally { rescanning = false; importDialog?.setRescanBusy(false); } } + /** + * D42: the concept's toast stays fixed at the bottom, so a discovery alert the rescan leaves is scrolled clear of + * the toast's band (its `scroll-margin-block-end`) only when it is in that band; elsewhere the page stays put. + */ + function announceRescan(message) { + showWorkspaceToast(message); + if (optionsStatus.hidden) return; + const {top, bottom} = optionsStatus.getBoundingClientRect(); + const band = parseFloat(getComputedStyle(optionsStatus).scrollMarginBlockEnd) || 0; + if (bottom > innerHeight - band && top < innerHeight) optionsStatus.scrollIntoView({block:'nearest', inline:'nearest'}); + } async function refreshInspectionCollection() { if (scope.signal.aborted) return; // Numeric roots may now name another source; retire the old stream first. diff --git a/web/js/workspace/media-viewer.mjs b/web/js/workspace/media-viewer.mjs index 1117a0a..4e8fc26 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({container,parentScope,inspectionController,sender,onSelect,onRefresh,onRestoreFocus,onClose,adapter = rootedFileAdapter}) { +export function createMediaViewer({api,container,parentScope,inspectionController,sender,onSelect,onRefresh,onRestoreFocus,onClose,adapter = rootedFileAdapter}) { const scope = createScope(); let active = null, retryVersion = 0; const dispose = () => scope.dispose(); @@ -80,7 +80,7 @@ export function createMediaViewer({container,parentScope,inspectionController,se const details = element('aside');details.id = 'viewer-details';details.setAttribute('aria-label','Image metadata'); body.append(left,details);dialog.append(header,body);container.append(dialog); const localSender = {open(roles,trigger) {session.stop();sender.open(roles,trigger);}}; - const inspector = mountFileInspector(details,{sender:localSender,parentScope:local,variant:'full',onInteraction:() => session.stop(),onRetry:() => inspectionController.retry(), + const inspector = mountFileInspector(details,{api,sender:localSender,parentScope:local,variant:'full',onInteraction:() => session.stop(),onRetry:() => inspectionController.retry(), onRefresh:onRefresh ? () => { closeViewer();onRefresh(); } : undefined}); function closeViewer(restore = true) { if (closed) return; diff --git a/web/js/workspace/prompt-blocks.mjs b/web/js/workspace/prompt-blocks.mjs index 9b069da..7f748f5 100644 --- a/web/js/workspace/prompt-blocks.mjs +++ b/web/js/workspace/prompt-blocks.mjs @@ -35,10 +35,10 @@ function childScope(parentScope) { export function mountPromptBlocks(container, {prompts, sender, parentScope, roles:visibleRoles = ['positive', 'negative'], onRetry, includePairAction = true, contextLabel = '', title = '', copyFallback = openCopyFallback}) { // A nested set of blocks (the current file inside a recorded run) names its regions and actions with a // context so assistive technology never meets two identically named prompt regions in one dialog. - const named = label => contextLabel ? `${contextLabel} ${label.toLowerCase()} prompt` : `${label} prompt`; + const named = roleLabel => contextLabel ? `${contextLabel} ${roleLabel.toLowerCase()} prompt` : `${roleLabel} prompt`; const action = role => contextLabel ? `${contextLabel[0].toLowerCase()}${contextLabel.slice(1)} ${role} prompt` : `${role} prompt`; - // The concept names a role's copy and send after the prompt they act on (app.js a298–302): "…: <title>". - const subject = title ? `: ${title}` : ''; + // A card among many names its actions after its prompt, as the concept's do (app.js a298–302: "Copy positive prompt: <title>"). + const actionName = (verb, role, rest = '') => `${verb} ${action(role)}${rest}${title ? `: ${title}` : ''}`; if (parentScope.signal.aborted) return {update() {}, setState() {}, dispose() {}}; const scope = childScope(parentScope); container.classList.add('prompt-inspector'); @@ -94,12 +94,12 @@ export function mountPromptBlocks(container, {prompts, sender, parentScope, role if (Object.hasOwn(roles, role)) { const actions = el('div', undefined, 'prompt-actions'); const copy = createButton('', 'icon-btn'); - copy.setAttribute('aria-label', `Copy ${action(role)}${subject}`); + copy.setAttribute('aria-label', actionName('Copy', role)); copy.append(createWorkspaceIcon('copy')); copy.dataset.copyRole = role; copy.disabled = !actionsEnabled; const send = createButton('', 'send icon-btn'); - send.setAttribute('aria-label', `Send ${action(role)} to ComfyUI${subject}`); + send.setAttribute('aria-label', actionName('Send', role, ' to ComfyUI')); send.append(createWorkspaceIcon('send')); send.dataset.sendRole = role; send.disabled = !actionsEnabled; diff --git a/web/js/workspace/workspace.css b/web/js/workspace/workspace.css index 7c6f48e..8668271 100644 --- a/web/js/workspace/workspace.css +++ b/web/js/workspace/workspace.css @@ -174,7 +174,9 @@ main:focus-visible { outline: 2px solid var(--ca-focus); outline-offset: 4px; } @container images-page (inline-size < 600px) { .image-workspace { grid-template-columns: minmax(0, 1fr); } } -.options-status { font-size: 12px; padding-inline: 4px; } +/* #249 D42: a discovery alert a rescan leaves scrolls clear of the concept's fixed toast (22px inset, 12px at 700px and + below, two 12px lines at 390px), never under it; the toast itself keeps the concept's placement. */ +.options-status { font-size: 12px; padding-inline: 4px; scroll-margin-block-end: 96px; /* D42 */ } /* Until their pages rebuild (PR F history): the history page's and the recorded run's prompt blocks keep their spacing (on the shelf's cards `.prompt-inspector` is `display: contents`, so its margin is inert there), the Archive negative dialog keeps `section.prompt-block`, and the shared prompt sender keeps its dialog and send button. */