diff --git a/TESTING.md b/TESTING.md index 939910b..d7cf4f4 100644 --- a/TESTING.md +++ b/TESTING.md @@ -590,6 +590,29 @@ 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 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 to `/prompt_manager/images/reveal` and toasts "Opened the + folder on this computer.", "Show in folder works only in a browser on the computer + running ComfyUI." (403 `not-local`), "No file manager is available on this + computer." (501 `unavailable`) or "The folder could not be opened. Try again." + The route is local-only: a loopback `request.remote`, none of `Forwarded`, + `X-Forwarded-For`, `X-Real-IP` or `X-Forwarded-Host`, and (against DNS rebinding) a + loopback `Host` and, when sent, `Origin` (`localhost`, `*.localhost` or a loopback + address). 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). `utils/file_reveal.py` starts the + opener as an argv list with an absolute executable, no shell, DEVNULL streams and + `close_fds`, never waited on: Windows `explorer.exe` from `%SystemRoot%` (never + PATH or the working directory) with `/select,` and the path, macOS `open -R`, + Linux the FileManager1 `ShowItems` dbus call (waited at most 2 s, only to detect a + failed start) and else `xdg-open` on the folder. `tests/test_image_reveal.py` mocks + `Popen`, `sys.platform` and `shutil.which`; the browser server's opener is a stub + (`POST`/`GET /__test__/reveal`). The inspector comparison reads closed disclosures + as hidden, so the button needs no mapping there. - **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 diff --git a/tests/browser/full-inspector.spec.mjs b/tests/browser/full-inspector.spec.mjs index 11ebd37..f5b7cef 100644 --- a/tests/browser/full-inspector.spec.mjs +++ b/tests/browser/full-inspector.spec.mjs @@ -242,7 +242,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(); @@ -270,6 +271,34 @@ 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'); + 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 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'); diff --git a/tests/browser/server.py b/tests/browser/server.py index e6c507d..9c8ba02 100644 --- a/tests/browser/server.py +++ b/tests/browser/server.py @@ -77,6 +77,51 @@ 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 + + state = {"outcome": "opened", "opened": []} + + def opener(path: 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 + + @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) @@ -112,6 +157,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] @@ -312,6 +358,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) @@ -410,6 +457,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( diff --git a/tests/js/workspace-file-inspector.test.js b/tests/js/workspace-file-inspector.test.js index d129814..ae97498 100644 --- a/tests/js/workspace-file-inspector.test.js +++ b/tests/js/workspace-file-inspector.test.js @@ -65,3 +65,23 @@ 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.'); + for(const error of [{status:403,code:null},{status:409,code:'source-changed'},{status:500},new TypeError('offline')]) + assert.equal(revealOutcome(error),'The folder could not be opened. Try again.'); +}); diff --git a/web/js/workspace/file-inspector.mjs b/web/js/workspace/file-inspector.mjs index 07ebf0d..323e9ec 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,12 @@ 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_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 +70,22 @@ 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; + return REVEAL_FAILED; +} + function el(tag, text, className) { const node = document.createElement(tag); if (text !== undefined) node.textContent = text; @@ -130,7 +153,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; @@ -236,13 +259,18 @@ 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([ + const facts = file && definitionList([ ['Source', file.source_label], ['Path', record.identity.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 (the second row). + const target = api ? revealTarget(record.identity) : null; + if (target) facts.children[1].lastChild.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 +286,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 478d4a2..29d480b 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -225,7 +225,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)); 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;