fix(images): show every Show in folder outcome above the viewer and report opener failures (#533)

Toasts raised from a modal were covered and inert, so every answer was invisible.
The toast now moves into the topmost open modal and back when it closes (D176).
xdg-open and open -R failures now answer 500 instead of success; Linux with no
display and no FileManager1 reply answers 501; xdg-open keeps its desktop
detection variables.

TESTING.md

@@ -1572,6 +1572,12 @@ Recorded deviations on this page (spec §8):
   | 429 `busy` | "The folder is already opening." |
   | Anything else (415, 404, 409, 500, offline) | "The folder could not be opened. Try again." |
 
+  **D176 (#533, beyond the concept):** the viewer is a modal, so `showWorkspaceToast`
+  moves `#workspace-toast` into the topmost open modal, keeping its fixed position,
+  and back when that modal closes. Otherwise every toast raised from a modal is
+  covered and inert while `toHaveText` still passes: assert `toBeVisible` and the
+  hit test as `full-inspector.spec.mjs` does.
+
   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
@@ -1596,7 +1602,9 @@ Recorded deviations on this page (spec §8):
   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
+  `xdg-open` on the folder, or answers 501 without `DISPLAY` and `WAYLAND_DISPLAY`.
+  `open` and `xdg-open` get `OPENER_WAIT` (1 s): a nonzero exit is the 500 above, and
+  one still running counts as started and goes to the reaper (#533). 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`,
@@ -1604,6 +1612,9 @@ Recorded deviations on this page (spec §8):
   (`POST`/`GET /__test__/reveal`) with no cooldown. The inspector comparison reads
   closed disclosures as hidden, so the button needs no mapping there.
 
+  LoRA Manager's open-location route has no local-only gate, so it also opens the
+  server's file manager for a proxied or remote browser; ours says it cannot.
+
   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

tests/browser/full-inspector.spec.mjs

@@ -363,6 +363,10 @@ test('Show in folder asks the server to open the listed file and announces each
     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.');
+    // #533: the answer is really on screen and reachable above the modal viewer, not covered by it.
+    await expect(toast).toBeVisible();
+    expect(await toast.evaluate(node=>{const box=node.getBoundingClientRect();const top=document.elementFromPoint(box.x+box.width/2,box.y+box.height/2);return top===node;})).toBe(true);
+    await expect(page.locator('#viewer-dialog').getByRole('status').filter({hasText:'Opened the folder on this computer.'})).toHaveCount(1);
     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.']]) {
@@ -377,7 +381,11 @@ test('Show in folder asks the server to open the listed file and announces each
     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.');
+    await expect(toast).toBeVisible();
     expect(await opened()).toEqual(['output/inspector/complete.png']);
+    // #533: closing the viewer takes the toast back to the page, still showing the message.
+    await page.keyboard.press('Escape');await expect(page.locator('#viewer-dialog')).toHaveCount(0);
+    await expect(toast).toBeVisible();await expect(page.locator('body > #workspace-toast')).toHaveCount(1);
 });
 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);

tests/js/workspace-toast.test.js

@@ -13,10 +13,11 @@ class FakeToast extends EventTarget {
     matches(selector) { assert.equal(selector, ':hover, :focus-within'); return this.held; }
 }
 
-function install() {
+function install(dialogs = []) {
     const toast = new FakeToast();
     const previous = global.document;
     global.document = {
+        querySelectorAll: selector => { assert.equal(selector, 'dialog'); return dialogs; },
         getElementById: id => id === 'workspace-toast' ? toast : null,
         createTextNode: text => ({ textContent: text }),
         createElement: tag => ({ tag, href: '', textContent: '' }),
@@ -24,6 +25,18 @@ function install() {
     return { toast, restore: () => { global.document = previous; } };
 }
 
+// First: the module remembers a toast it has shown.
+test('a page without the toast shows nothing and does not fail', async () => {
+    const { showWorkspaceToast } = await source;
+    const previous = global.document;
+    global.document = { getElementById: () => null };
+    try {
+        assert.doesNotThrow(() => showWorkspaceToast('Nothing.'));
+    } finally {
+        global.document = previous;
+    }
+});
+
 test('a plain toast clears after 6.5 s and a linked one after 20 s', async t => {
     const { showWorkspaceToast, TOAST_MS, LINK_TOAST_MS } = await source;
     t.mock.timers.enable({ apis: ['setTimeout'] });
@@ -84,13 +97,50 @@ test('a toast under the pointer or focus waits until they leave', async t => {
     }
 });
 
-test('a page without the toast shows nothing and does not fail', async () => {
+
+class FakeHost extends EventTarget {
+    constructor(modal = false, localName = 'dialog') { super(); this.modal = modal; this.localName = localName; this.nodes = []; }
+    matches(selector) { assert.equal(selector, ':modal'); return this.modal; }
+    append(node) { node.parentNode?.nodes.splice(node.parentNode.nodes.indexOf(node), 1); node.parentNode = this; this.nodes.push(node); }
+}
+
+test('a toast shows inside the topmost open modal and goes home when it closes (#533)', async () => {
     const { showWorkspaceToast } = await source;
-    const previous = global.document;
-    global.document = { getElementById: () => null };
+    const closed = new FakeHost(false), lower = new FakeHost(true), top = new FakeHost(true);
+    const dialogs = [closed, lower, top];
+    const { toast, restore } = install(dialogs);
+    const home = new FakeHost(false, 'body');home.append(toast);
     try {
-        assert.doesNotThrow(() => showWorkspaceToast('Nothing.'));
+        showWorkspaceToast('Opened the folder on this computer.');
+        assert.equal(toast.parentNode, top);
+        // The upper modal closes: the toast stays visible in the one under it.
+        top.modal = false;top.dispatchEvent(new Event('close'));
+        assert.equal(toast.parentNode, lower);
+        lower.modal = false;lower.dispatchEvent(new Event('close'));
+        assert.equal(toast.parentNode, home);
+        showWorkspaceToast('Next.');
+        assert.equal(toast.parentNode, home);
     } finally {
-        global.document = previous;
+        restore();
+    }
+});
+
+test('a toast removed along with its modal still shows the next message (#533)', async () => {
+    const { showWorkspaceToast } = await source;
+    const modal = new FakeHost(true);
+    const { toast, restore } = install([modal]);
+    const home = new FakeHost(false, 'body');home.append(toast);
+    try {
+        showWorkspaceToast('Inside.');
+        assert.equal(toast.parentNode, modal);
+        modal.modal = false;
+        const byId = document.getElementById;
+        document.getElementById = () => null; // the dialog left the page with the toast in it
+        showWorkspaceToast('After.');
+        assert.equal(toast.parentNode, home);
+        assert.equal(toast.textContent, 'After.');
+        document.getElementById = byId;
+    } finally {
+        restore();
     }
 });

tests/test_image_reveal.py

@@ -171,7 +171,10 @@ class TestRevealInFileManager(unittest.TestCase):
         self.path.parent.mkdir(parents=True)
         self.path.write_bytes(b"png")
         self.popen = patch.object(subprocess, "Popen").start()
+        self.popen.return_value.wait.return_value = 0
         self.reaped = patch.object(file_reveal, "_reap").start()
+        # A desktop session unless a test says otherwise; CI has no display.
+        patch.dict(os.environ, {"DISPLAY": ":0"}).start()
         self.addCleanup(patch.stopall)
 
     def reveal(self, path=None):
@@ -262,8 +265,72 @@ class TestRevealInFileManager(unittest.TestCase):
             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)
+        self.popen.return_value.wait.assert_called_once_with(
+            timeout=file_reveal.OPENER_WAIT
+        )
+        self.reaped.assert_not_called()
+
+    def test_a_failing_opener_is_reported_on_macos_and_linux(self):
+        cases = {
+            "darwin": patch.object(
+                file_reveal, "_macos_open", return_value="/usr/bin/open"
+            ),
+            "linux": self.which({"xdg-open": "/usr/bin/xdg-open"}),
+        }
+        for platform, opener in cases.items():
+            with self.subTest(platform):
+                self.popen.return_value.wait.return_value = 4
+                with (
+                    patch.object(file_reveal.sys, "platform", platform),
+                    opener,
+                    self.assertRaises(file_reveal.OpenerFailed),
+                ):
+                    self.reveal()
+                self.reaped.assert_not_called()
+
+    def test_an_opener_still_running_counts_as_started_and_is_reaped(self):
+        cases = {
+            "darwin": patch.object(
+                file_reveal, "_macos_open", return_value="/usr/bin/open"
+            ),
+            "linux": self.which({"xdg-open": "/usr/bin/xdg-open"}),
+        }
+        for platform, opener in cases.items():
+            with self.subTest(platform):
+                self.reaped.reset_mock()
+                process = self.popen.return_value
+                process.wait.side_effect = subprocess.TimeoutExpired("opener", 1)
+                with patch.object(file_reveal.sys, "platform", platform), opener:
+                    self.reveal()
+                self.reaped.assert_called_once_with(process)
+
+    def test_linux_without_a_display_or_a_service_is_unavailable(self):
+        with (
+            patch.object(file_reveal.sys, "platform", "linux"),
+            patch.dict(os.environ, {"DISPLAY": "", "WAYLAND_DISPLAY": ""}),
+            self.which({"xdg-open": "/usr/bin/xdg-open"}),
+            self.assertRaises(OpenerUnavailable),
+        ):
+            self.reveal()
+        self.popen.assert_not_called()
+
+    def test_linux_on_wayland_alone_still_runs_xdg_open(self):
+        with (
+            patch.object(file_reveal.sys, "platform", "linux"),
+            patch.dict(os.environ, {"DISPLAY": "", "WAYLAND_DISPLAY": "wayland-0"}),
+            self.which({"xdg-open": "/usr/bin/xdg-open"}),
+        ):
+            self.reveal()
+        self.assertEqual(self.argv(), ["/usr/bin/xdg-open", str(self.path.parent)])
+
+    def test_the_service_answers_without_needing_a_display(self):
+        with (
+            patch.object(file_reveal.sys, "platform", "linux"),
+            patch.dict(os.environ, {"DISPLAY": "", "WAYLAND_DISPLAY": ""}),
+            self.which({"dbus-send": "/usr/bin/dbus-send"}),
+        ):
+            self.reveal()
+        self.assertEqual(self.popen.call_count, 1)
 
     def test_macos_open_is_the_fixed_system_path(self):
         self.assertEqual(file_reveal.MACOS_OPEN, "/usr/bin/open")
@@ -324,6 +391,12 @@ class TestRevealInFileManager(unittest.TestCase):
             "XDG_SESSION_DESKTOP": "KDE",
             "XDG_CONFIG_HOME": "/home/u/.config",
             "XDG_DATA_DIRS": "/usr/share",
+            # What xdg-open reads to choose an opener (kde-open6 on Plasma 6).
+            "KDE_FULL_SESSION": "true",
+            "KDE_SESSION_VERSION": "6",
+            "GNOME_DESKTOP_SESSION_ID": "this-is-deprecated",
+            "MATE_DESKTOP_SESSION_ID": "this-is-mate",
+            "LXQT_SESSION_CONFIG": "session",
         }
         with patch.dict(os.environ, inherited, clear=True):
             env = file_reveal._child_env()
@@ -343,6 +416,11 @@ class TestRevealInFileManager(unittest.TestCase):
                 "XDG_SESSION_DESKTOP": "KDE",
                 "XDG_CONFIG_HOME": "/home/u/.config",
                 "XDG_DATA_DIRS": "/usr/share",
+                "KDE_FULL_SESSION": "true",
+                "KDE_SESSION_VERSION": "6",
+                "GNOME_DESKTOP_SESSION_ID": "this-is-deprecated",
+                "MATE_DESKTOP_SESSION_ID": "this-is-mate",
+                "LXQT_SESSION_CONFIG": "session",
             },
         )
         self.assertNotIn("/venv", file_reveal.SYSTEM_PATH)
@@ -387,15 +465,19 @@ class TestRevealInFileManager(unittest.TestCase):
         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()
+                opener = MagicMock()
+                opener.wait.return_value = 0
                 if isinstance(outcome, OSError):
-                    self.popen.side_effect = [outcome, MagicMock()]
+                    self.popen.side_effect = [outcome, opener]
                 elif isinstance(outcome, subprocess.TimeoutExpired):
                     dbus.wait.side_effect = [outcome, None]
-                    self.popen.side_effect = [dbus, MagicMock()]
+                    self.popen.side_effect = [dbus, opener]
+                elif outcome is None:
+                    self.popen.side_effect = [opener]
                 else:
-                    self.popen.return_value.wait.return_value = outcome
+                    dbus.wait.return_value = outcome
+                    self.popen.side_effect = [dbus, opener]
                 with (
                     patch.object(file_reveal.sys, "platform", "linux"),
                     self.which(available),
@@ -406,7 +488,7 @@ class TestRevealInFileManager(unittest.TestCase):
                     ["/usr/bin/xdg-open", str(self.path.parent)],
                 )
                 self.assert_detached(self.popen.call_count - 1)
-                self.reaped.assert_called_once()
+                opener.wait.assert_called_once_with(timeout=file_reveal.OPENER_WAIT)
                 if name == "hung":
                     dbus.kill.assert_called_once_with()
 

utils/file_reveal.py

@@ -51,6 +51,9 @@ REVEAL_COOLDOWN = 1.0
 DBUS_REPLY_TIMEOUT_MS = 2000
 # The dbus-send process itself gets a little longer before it is killed.
 DBUS_WAIT = 3.0
+# An opener that fails says so at once; one still running after this long has
+# started a file manager (it may stay in the foreground).
+OPENER_WAIT = 1.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.
@@ -77,6 +80,12 @@ CHILD_ENV_NAMES = (
     "DBUS_SESSION_BUS_ADDRESS",
     "LANG",
     "LC_ALL",
+    # What xdg-open reads to choose an opener (kde-open6 on Plasma 6).
+    "KDE_FULL_SESSION",
+    "KDE_SESSION_VERSION",
+    "GNOME_DESKTOP_SESSION_ID",
+    "MATE_DESKTOP_SESSION_ID",
+    "LXQT_SESSION_CONFIG",
 )
 
 
@@ -84,6 +93,10 @@ class OpenerUnavailable(Exception):
     """No file manager can be started on this computer."""
 
 
+class OpenerFailed(Exception):
+    """The file manager's opener ran and reported a failure."""
+
+
 class RevealTargetChanged(Exception):
     """The file is no longer a regular file inside its root."""
 
@@ -393,6 +406,19 @@ def _launch(argv: list[str]) -> subprocess.Popen:
     return subprocess.Popen(argv, **options)  # nosec B603
 
 
+def _launch_opener(argv: list[str]) -> None:
+    """Start an opener and report its failure (OpenerFailed), never waiting on
+    a file manager it stays attached to: that one is left to the reaper."""
+    process = _launch(argv)
+    try:
+        status = process.wait(timeout=OPENER_WAIT)
+    except subprocess.TimeoutExpired:
+        _reap(process)
+        return
+    if status != 0:
+        raise OpenerFailed(f"exit status {status}")
+
+
 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)."""
@@ -427,8 +453,9 @@ 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.
+    OpenerUnavailable when no opener exists or Linux has no graphical session,
+    OSError when the one found cannot start and OpenerFailed when it reports a
+    failure. Never waits on the file manager itself.
     """
     path = Path(path)
     if not path.is_absolute():
@@ -441,11 +468,12 @@ def reveal_in_file_manager(path: Path, root: Path) -> None:
         opener = _macos_open()
         if opener is None:
             raise OpenerUnavailable()
-        _reap(_launch([opener, "-R", str(path)]))
+        _launch_opener([opener, "-R", str(path)])
         return
     if _show_items(path):
         return
     opener = _executable("xdg-open")
-    if opener is None:
+    has_display = os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY")
+    if opener is None or not has_display:
         raise OpenerUnavailable()
-    _reap(_launch([opener, str(path.parent)]))
+    _launch_opener([opener, str(path.parent)])

web/js/workspace/toast.mjs

@@ -5,6 +5,28 @@ export const LINK_TOAST_MS = 20000;
 let toastTimer;
 // Stops waiting for the pointer or focus to leave a toast that a newer one replaced.
 let releaseWait = null;
+// The toast and the page element it lives in when no modal is open.
+let toastElement = null;
+let toastHome = null;
+
+/** The open modal the user is in: the last in the page, as a nested dialog is opened after its parent. */
+function topModal() {
+    return [...document.querySelectorAll('dialog')].filter(dialog => dialog.matches(':modal')).at(-1) ?? null;
+}
+
+/**
+ * #533: a modal covers and inerts the rest of the page, so the toast shows inside the topmost one (D176) and
+ * follows it out when it closes. Called by that dialog's `close` too; the same listener is only ever added once.
+ */
+function placeToast() {
+    const toast = toastElement;
+    const parent = toast.parentNode;
+    if (parent && parent.localName !== 'dialog') toastHome = parent;
+    const modal = topModal();
+    const host = modal ?? toastHome;
+    if (host && parent !== host) host.append(toast);
+    modal?.addEventListener('close', placeToast);
+}
 
 /** Whether the pointer or keyboard focus rests on the toast. */
 const held = toast => typeof toast.matches === 'function' && toast.matches(':hover, :focus-within');
@@ -41,8 +63,11 @@ function scheduleClear(toast, delay) {
  * for LINK_TOAST_MS and while the pointer or focus is on it.
  */
 export function showWorkspaceToast(message) {
-    const toast = document.getElementById('workspace-toast');
+    // Null only when a dialog left the page with the toast still in it.
+    const toast = document.getElementById('workspace-toast') ?? toastElement;
     if (!toast) return;
+    toastElement = toast;
+    placeToast();
     releaseWait?.();
     if (Array.isArray(message)) {
         toast.replaceChildren(...message.map(part => {