From 3ae1bd0538e05bb269633f3aefd2d9baa30a416a Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Fri, 23 Oct 2026 12:08:55 -0700 Subject: [PATCH] fix(system): say what Sources saved, refused and still watches - A per-source scan sends the source's public path; when the sources changed since they were read the toast says the server's reason and the list and saved values are read again. - "watching until restart" for a source the running monitor still watches after monitoring was saved off, and the status adds "Restart ComfyUI to apply: Watch for new outputs." from restart_required_fields. - A save refused for one unusable source marks that row and the status is the server's message naming it. - A valid configured source the effective list holds under another spelling reads "same folder as another source". - The busy toast names the job the 409 named (ApiError keeps a validated running job and refused source), falling back to the list. - The Add source dialog's listeners live on the page scope and Enter adds; its markup is compared with the concept (D127). - TESTING.md: D-NEW-S4-1 is D127; D-NEW-S4-2 records the new copy; the Sources contract describes the lexical containment, invalid-source and sources-changed. --- TESTING.md | 67 +++++++++---- tests/browser/system-concept.spec.mjs | 94 ++++++++++++++++++- .../browser/workspace-system-sources.spec.mjs | 78 +++++++++++++-- tests/browser/workspace_system_fixture.py | 27 +++++- tests/js/api-client.test.js | 17 ++++ tests/js/workspace-system-sources.test.js | 30 +++++- tests/js/workspace-system-state.test.js | 48 ++++++++++ tools/vendor/frontend-coverage-baseline.json | 4 +- web/js/api-client.js | 15 +++ web/js/workspace/activity-scan.mjs | 30 ++++-- web/js/workspace/system-sources.mjs | 60 ++++++++---- web/js/workspace/system-state.mjs | 37 +++++++- web/js/workspace/system.mjs | 9 +- web/js/workspace/workspace.css | 2 + 14 files changed, 446 insertions(+), 72 deletions(-) diff --git a/TESTING.md b/TESTING.md index 054dde4b..71bea14f 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1876,20 +1876,36 @@ Sources & scans (`system-sources.mjs`, S4) is the concept's "sources" body on th server's sources. `GET /prompt_manager/settings` reports them (public paths only): `sources.configured` (each `gallery_root_paths` entry with `valid`, `code` and the server's `reason`), `sources.effective` (what is scanned and served, with its -`root_index` and `watch`: `watching` only when the image monitor's observer is alive -and scheduled that folder, `manual` when monitoring is off or no monitor runs, else +`root_index` and `watch`: `watching` only when the image monitor's observer is alive, +scheduled that folder and monitoring is on, `stopping` when it still watches it but +monitoring was switched off, `manual` when monitoring is off or no monitor runs, else `restart_pending`), `sources.auto_detected`, `monitor` and `settings.monitoring_enabled`. `POST /prompt_manager/settings/validate-source {path}` checks one folder and writes -nothing; a path outside the allowed ComfyUI folders answers `outside-allowed` with one -reason whether it exists or not (no existence oracle). A save that changes the sources -holds the registry as `records` ("Source change"), so it is refused with 409 `busy` -while a scan runs. `POST /prompt_manager/jobs/scan {scope, roots}` scans only the given -effective `root_index`es (strict ints, at most 64; an unknown one is 400 -`unknown-source` and starts nothing); the legacy `/scan` is unchanged. The section -lists one row per source (" · watching | manual scan | restart pending" with -"Scan changes" for a watched source, else "Scan folder"), which opens the shared -"Review metadata scan" for that one source ("1 output folder") and starts the session -job over it; the job opens in Activity, and the toast only says it started. Enrich +nothing. Every caller (validate-source, the settings save, config.json at load) refuses +a source through the same checks (`sources.source_problem`): its form first, then +containment decided as text (`normcase(normpath)` against the allowed folders' own +spellings), before anything resolves, stats or opens the path, so a UNC share, the +`\\?\` and `\\.\` namespaces, a device or any folder outside the allowed ones never +reaches the filesystem or the network (no SMB request, no existence or timing oracle). +Only then is the path resolved and contained again, which catches a symlink out. The +filesystem root and the home directory answer `outside-allowed` with the same reason as +any outside path. ComfyUI folders outside its base directory (`--output-directory`) +are path anchors too, so their sources keep a relative public path +(`renders/portraits`) that round-trips and never collides by folder name. A save whose +list holds a source the server cannot use is a 400 `invalid-source` naming its public +path (`source`, as text: a refused path is never resolved to name it), its `index` and +`reason`, and saves nothing. A save that changes the sources (compared as folders, not +spellings) holds the registry as `records` ("Source change"), so it is refused with +409 `busy` while a scan runs. `POST /prompt_manager/jobs/scan {scope, roots, paths}` +scans only the given effective `root_index`es (strict ints, at most 64), each with the +public path the page read for it; an index past the list or one that now names another +folder is 409 `sources-changed` and starts nothing (the page toasts the server's +reason and reads the sources again); the legacy `/scan` is unchanged. The section +lists one row per source (" · watching | watching until restart | manual scan | +restart pending" with "Scan changes" for a watched source, else "Scan folder"), which +opens the shared "Review metadata scan" for that one source ("1 output folder") and +starts the session job over it; the job opens in Activity, and the toast only says it +started. Enrich existing metadata and the notice's Review scan review every source. The notice shows until this browser dismissed the 3.2.0 update notice (`pm_update_notice_dismissed`, the legacy admin page's key); Later dismisses it without scanning, and Review scan @@ -1898,8 +1914,10 @@ the folder to the host draft of `gallery_root_paths`; Remove takes one out; "Wat new outputs" (`monitoring_enabled`) and Worker threads (`worker_threads`) are host drafts too, and Save writes them with the revision. A save the server refuses because a scan runs toasts "Another job is running: ." with a link to that job (D57) and -keeps the drafts. After a save the source list is read again; a monitoring change waits -for a restart (the running observer keeps watching until then). Thumbnail generation's +keeps the drafts; a save refused for one source says the server's message as the status +and marks that row. After a save the source list is read again; a monitoring change +waits for a restart (the running observer keeps watching until then, and the status +says "Restart ComfyUI to apply: Watch for new outputs."). Thumbnail generation's Review options opens the legacy gallery in a new tab (D94) until its review dialog (S5). A list that cannot be read shows "Sources could not be loaded. Your saved data has not changed." and Retry, never an empty list. @@ -1931,7 +1949,7 @@ the connection's `total_changes` is 0 at close and the data reads back unchanged `-shm` bytes are not compared: every WAL reader rewrites its read marks there, so no reader can keep it byte-identical (`test_health_check_never_writes`). -Recorded deviations (plan D60–D66 and D81 renumbered D92–D98 and D113; D121, D122, D123 and D124 assigned; D-NEW-S4-1 awaits its number): +Recorded deviations (plan D60–D66 and D81 renumbered D92–D98 and D113; D121–D124 and D127 assigned; D-NEW-S4-2 awaits its number): - **D92** (shell): no "Sample configuration" chip. The Save status is truthful: "No unsaved changes.", "Unsaved changes.", "Saved in this browser." / "Saved to the server @@ -2014,16 +2032,31 @@ Recorded deviations (plan D60–D66 and D81 renumbered D92–D98 and D113; D121, - **D113** (Sources): the Worker threads range ends at the server's `cpu_count` (the concept's fixed `max=16`) and shows the server's real `worker_threads` (the concept's sample 4). -- **D-NEW-S4-1** (Sources): the concept's `add-source` `simpleInput` with production +- **D127** (Sources): the concept's `add-source` `simpleInput` with production copy: the field is "Folder path" (the concept's "(sample only)" dropped), it starts empty (not the demo "/images/new-collection"), and the action is "Add" (not "Apply in - prototype"). A folder the server refuses keeps the dialog open with the field + prototype"); Enter in the field adds too. `add source dialog matches the concept` + (system-concept.spec) compares the open dialog with these three mapped, and `add + source comparison detects mutations` proves the mapping is the only difference. A + folder the server refuses keeps the dialog open with the field `aria-invalid` and the server's reason in a `p.small[role=alert]` above the actions; a folder the server names as one already listed says "This folder is already a source.", and a check that cannot finish says "The folder could not be checked. Try again."; neither adds anything. The concept dialog does no check, so `invalid source shows the server reason` (workspace-system-sources.spec) pins it. +- **D-NEW-S4-2** (Sources): states the concept's sample rows never show. A source the + running monitor still watches after "Watch for new outputs" was saved off reads + "<path> · watching until restart" (still "Scan changes"), and the save's status adds + "Restart ComfyUI to apply: Watch for new outputs." from `restart_required_fields`. A + configured source the effective list holds under another spelling reads "<path> · + same folder as another source" with only Remove. A save refused for one source + (`invalid-source`) marks that row (`aria-invalid`, outlined like a D92 setting) and + the status is the server's "<path> can't be used: <reason>. Remove it or reconnect it, + then save." ("Remove it, then save." when reconnecting cannot help). Pinned by + `Watch for new outputs saves monitoring_enabled; a restart applies it` and `a save + refused for one unusable source names it and marks its row` + (workspace-system-sources.spec) and the system-state and system-sources node tests. - **D124** (inspector): with "Load metadata on selection" off, the Images panel and the viewer's details show "Metadata loads when you ask for it." and a "Load metadata" button instead of reading the file. The concept has no such state, so no concept diff --git a/tests/browser/system-concept.spec.mjs b/tests/browser/system-concept.spec.mjs index 1b21809c..b373057c 100644 --- a/tests/browser/system-concept.spec.mjs +++ b/tests/browser/system-concept.spec.mjs @@ -584,7 +584,9 @@ function measureSources(mainSelector) { }; } -const WATCH_TEXT = { watching: 'watching', manual: 'manual scan', restart_pending: 'restart pending' }; +// D-NEW-S4-2: "watching until restart" after monitoring was switched off while the observer still runs. +const WATCH_TEXT = { watching: 'watching', stopping: 'watching until restart', manual: 'manual scan', restart_pending: 'restart pending' }; +const WATCHED = new Set(['watching', 'stopping']); // D98: the operation Sources adds after the concept's own, as production renders it. const REFRESH_FILES = { title: 'Refresh files', description: 'Read every source again to find files added or removed outside ComfyUI.', label: 'Refresh' }; @@ -599,7 +601,7 @@ function mapSourcesConcept(reference, { settings, sources }) { // a plain button after its scan. sources: sources.effective.map(source => ({ ...sample, title: source.label, description: `${source.path} · ${WATCH_TEXT[source.watch]}`, - actions: [{ ...scan, text: source.watch === 'watching' ? 'Scan changes' : 'Scan folder' }, { ...scan, text: 'Remove' }] })), + actions: [{ ...scan, text: WATCHED.has(source.watch) ? 'Scan changes' : 'Scan folder' }, { ...scan, text: 'Remove' }] })), rows: reference.rows.map(row => { if (row.control.id === 'monitoring') return { ...row, control: { ...row.control, checked: settings.monitoring_enabled } }; // D113: the range ends at the server's core count (the concept's fixed 16) and shows the real worker count. @@ -686,3 +688,91 @@ test('sources comparison detects mutations', async ({ page, request }) => { await undoToken(); expect(diffInventory(expected, await measure())).toEqual([]); }); + +/** + * Runs in the page: the open "Add image source" dialog (the concept's `simpleInput`, app.js 1138 and 1497): its title, + * the body's tree, the field's label and input, and the actions, with the concept's geometry and tokens. + */ +function measureAddSource() { + const dialog = document.querySelector('dialog#modal[open]'); + const css = (node, ...keys) => node ? Object.fromEntries(keys.map(key => [key, getComputedStyle(node)[key]])) : null; + const name = node => [node.localName, ...[...node.classList].sort()].join('.'); + const text = node => node?.textContent.trim() ?? null; + const body = dialog.querySelector('#modal-body'); + const field = body.querySelector(':scope > .field'); + const label = field?.querySelector(':scope > label'); + const input = field?.querySelector(':scope > input'); + const actions = body.querySelector(':scope > .dialog-actions'); + const rect = dialog.getBoundingClientRect(); + return { + title: text(dialog.querySelector('#modal-title')), + tree: [...body.children].map(name), + field: field && { tree: [...field.children].map(name), ...css(field, 'display', 'flexDirection', 'rowGap', 'marginBottom') }, + label: label && { text: text(label), for: label.htmlFor, ...css(label, 'fontSize', 'fontWeight', 'color') }, + input: input && { id: input.id, type: input.type, value: input.value, + ...css(input, 'height', 'paddingLeft', 'fontSize', 'color', 'backgroundColor', 'borderTopColor', 'borderTopLeftRadius') }, + actions: { tree: [...actions.children].map(name), ...css(actions, 'display', 'justifyContent', 'columnGap', 'marginTop'), + buttons: [...actions.children].map(button => ({ text: text(button), className: button.className, + ...css(button, 'backgroundColor', 'color', 'fontSize', 'paddingTop', 'paddingLeft', 'borderTopColor') })) }, + width: Math.round(rect.width), + }; +} + +/** D127: the field is "Folder path", starts empty (no sample folder) and its action is "Add". */ +function mapAddSourceConcept(reference) { + const [cancel, apply] = reference.actions.buttons; + return { ...reference, label: { ...reference.label, text: 'Folder path' }, input: { ...reference.input, value: '' }, + actions: { ...reference.actions, buttons: [cancel, { ...apply, text: 'Add' }] } }; +} + +async function conceptAddSource(page, theme) { + await conceptSources(page, theme, { notice: false }); + await page.locator('#main .panel-heading').getByRole('button', { name: '+ Add source', exact: true }).click(); + await expect(page.locator('dialog#modal[open] #modal-title')).toHaveText('Add image source'); + await page.mouse.move(0, 0); + return page.evaluate(measureAddSource); +} + +async function productionAddSource(page, theme) { + await productionSources(page, theme, { notice: false }); + await page.locator('#workspace-main .panel-heading').getByRole('button', { name: '+ Add source', exact: true }).click(); + await expect(page.locator('dialog#modal[open] #modal-title')).toHaveText('Add image source'); + await page.mouse.move(0, 0); + return page.evaluate(measureAddSource); +} + +for (const theme of THEMES) for (const [width, height] of SIZES) { + test(`add source dialog matches the concept ${theme} ${width}x${height}`, async ({ page, request }, testInfo) => { + await page.setViewportSize({ width, height }); + await watchedSources(request); + const concept = await conceptAddSource(page, theme); + await page.screenshot({ path: testInfo.outputPath(`concept-add-source-${theme}-${width}.png`) }); + const production = await productionAddSource(page, theme); + await page.screenshot({ path: testInfo.outputPath(`production-add-source-${theme}-${width}.png`) }); + expect(diffInventory(mapAddSourceConcept(concept), production)).toEqual([]); + }); +} + +test('add source comparison detects mutations', async ({ page, request }) => { + await page.setViewportSize({ width: 1600, height: 1100 }); + await watchedSources(request); + const concept = await conceptAddSource(page, 'dark'); + const expected = mapAddSourceConcept(concept); + const production = await productionAddSource(page, 'dark'); + expect(diffInventory(expected, production)).toEqual([]); + // Unmapped, the concept differs only where D127 is recorded. + expect(diffInventory(concept, production).map(item => item.path).sort()) + .toEqual(['actions.buttons.1.text', 'input.value', 'label.text']); + const paths = async () => diffInventory(expected, await page.evaluate(measureAddSource)).map(item => item.path); + // A changed label, a swapped action order and a changed style. + await page.evaluate(() => { document.querySelector('dialog#modal[open] label[for="action-input"]').textContent = 'Folder'; }); + expect(await paths()).toEqual(['label.text']); + await page.evaluate(() => { document.querySelector('dialog#modal[open] label[for="action-input"]').textContent = 'Folder path'; }); + await page.evaluate(() => { const actions = document.querySelector('dialog#modal[open] .dialog-actions'); actions.prepend(actions.lastElementChild); }); + expect(await paths()).toEqual(expect.arrayContaining(['actions.buttons.0.text', 'actions.buttons.1.text'])); + await page.evaluate(() => { const actions = document.querySelector('dialog#modal[open] .dialog-actions'); actions.prepend(actions.lastElementChild); }); + const undoStyle = await mutate(page, { css: 'dialog#modal #action-input { border-color: rgb(255, 0, 0); } dialog#modal .dialog-actions { gap: 31px; }' }); + expect(await paths()).toEqual(expect.arrayContaining(['input.borderTopColor', 'actions.columnGap'])); + await undoStyle(); + expect(diffInventory(expected, await page.evaluate(measureAddSource))).toEqual([]); +}); diff --git a/tests/browser/workspace-system-sources.spec.mjs b/tests/browser/workspace-system-sources.spec.mjs index d7b3a9fb..a0d2c73a 100644 --- a/tests/browser/workspace-system-sources.spec.mjs +++ b/tests/browser/workspace-system-sources.spec.mjs @@ -112,7 +112,7 @@ test('Scan changes opens it with that one source ("1 output folder") and starts await expect(page).toHaveURL(/#\/activity\/[0-9a-f]{32}$/); // The toast says it started (or, for a scan this quick, that it already finished); Activity shows its progress. await expect(toast(page)).toHaveText(/^Scan (started\.|finished · )/); - expect(bodies).toEqual([{ scope: 'new', roots: [1] }]); + expect(bodies).toEqual([{ scope: 'new', roots: [1], paths: ['input'] }]); const id = page.url().split('/').pop(); // The job walks only that source (the count is recorded once the walk has listed it). await expect.poll(async () => (await serverJson(request, `/prompt_manager/jobs/${id}`)).job.scope) @@ -165,12 +165,14 @@ test('invalid source shows the server reason', async ({ page, request }) => { const layout = await openSources(page); await layout.getByRole('button', { name: '+ Add source', exact: true }).click(); const field = dialog(page).locator('#action-input'); - for (const [path, reason] of [ - ['output/not-there', 'Gallery root does not exist'], - ['/etc', OUTSIDE_REASON], + for (const [path, reason, submit] of [ + ['output/not-there', 'Gallery root does not exist', 'click'], + // Enter in the field adds too (M6). + ['/etc', OUTSIDE_REASON, 'Enter'], ]) { await field.fill(path); - await dialog(page).getByRole('button', { name: 'Add', exact: true }).click(); + if (submit === 'Enter') await field.press('Enter'); + else await dialog(page).getByRole('button', { name: 'Add', exact: true }).click(); await expect(dialog(page).locator('p.small[role="alert"]')).toHaveText(reason); await expect(field).toHaveAttribute('aria-invalid', 'true'); await expect(field).toHaveValue(path); @@ -253,10 +255,17 @@ test('Watch for new outputs saves monitoring_enabled; a restart applies it', asy await saveButton(page).click(); const answer = await (await saved).json(); expect(answer.restart_required_fields).toEqual(['monitoring_enabled']); - await expect(statusOf(page)).toHaveText('Saved to the server configuration.'); - // Saved for the next start: the running observer keeps watching until ComfyUI restarts, and the list says so. + // D-NEW-S4-2: saved for the next start, and the status says a restart applies it. + await expect(statusOf(page)).toHaveText('Saved to the server configuration. Restart ComfyUI to apply: Watch for new outputs.'); + // The running observer keeps watching until ComfyUI restarts, and the list says so. const { settings, monitor } = await serverJson(request, SETTINGS_URL); expect([settings.monitoring_enabled, monitor.observer_alive]).toEqual([false, true]); + await expect(sourceList(page).locator('.operation p')).toHaveText(['output · watching until restart', 'input · watching until restart']); + await expect(sourceList(page).locator('.operation .row-actions > button:first-child')).toHaveText(['Scan changes', 'Scan changes']); + // Switched back on before the restart: the observer still runs, so nothing waits for one. + await monitoring.check(); + await saveButton(page).click(); + await expect(statusOf(page)).toHaveText('Saved to the server configuration.'); await expect(sourceList(page).locator('.operation p')).toHaveText(['output · watching', 'input · watching']); }); @@ -318,6 +327,15 @@ test('a save refused by a running scan shows the busy link toast', async ({ page // The draft is kept for the next Save, and the server still has both sources. await expect(sourceList(page).locator('.operation')).toHaveCount(1); expect((await serverJson(request, SETTINGS_URL)).sources.configured).toHaveLength(2); + // An answer that does not name its job (an older server): the toast names the running one from the job list. + await page.route(`**${SETTINGS_URL}`, route => (route.request().method() === 'POST' + ? route.fulfill({ status: 409, json: { success: false, code: 'busy', error: 'Scan for metadata already running' } }) + : route.continue())); + await page.locator('#workspace-toast').evaluate(node => { node.textContent = ''; }); + await saveButton(page).click(); + await expect(toast(page)).toHaveText('Another job is running: Scan for metadata.'); + await expect(toast(page).getByRole('link', { name: 'Scan for metadata', exact: true })).toHaveAttribute('href', `#/activity/${running.job.id}`); + await page.unroute(`**${SETTINGS_URL}`); await request.post(`/__test__/jobs/${running.job.id}/finish`, { data: { state: 'finished' } }); }); @@ -335,3 +353,49 @@ test('a source list that cannot be read says so and retries', async ({ page }) = await sourceList(page).getByRole('button', { name: 'Retry', exact: true }).click(); await expect(sourceList(page).locator('.operation')).toHaveCount(2); }); + +test('a scan from a list that changed since it was read starts nothing, says why and reads the list again', async ({ page, request }) => { + await openSources(page); + // The legacy page (or another tab) removes ComfyUI output: the page's "input" is index 1, now past the end. + await expect(await request.post(SETTINGS_URL, { data: { gallery_root_paths: ['input'] } })).toBeOK(); + const refused = page.waitForResponse(response => response.url().endsWith('/prompt_manager/jobs/scan')); + await sourceRow(page, 'ComfyUI input').getByRole('button', { name: 'Scan folder', exact: true }).click(); + await dialog(page).getByRole('button', { name: 'Start scan', exact: true }).click(); + const answer = await refused; + expect([answer.status(), (await answer.json()).code]).toEqual([409, 'sources-changed']); + await expect(toast(page)).toHaveText('The sources changed since they were loaded. Reload them.'); + // No scan started (the other save ran as its own source-change hold). + expect((await serverJson(request, '/prompt_manager/jobs')).jobs.map(job => job.kind)).toEqual(['source_change']); + // Read again: the one source is index 0 now, and its scan starts. + await expect.poll(rowsOf.bind(null, page)).toEqual([['ComfyUI input', 'input · manual scan', ['Scan folder', 'Remove']]]); + await expect(statusOf(page)).toHaveText('No unsaved changes.'); + const started = page.waitForRequest(item => item.method() === 'POST' && item.url().endsWith('/prompt_manager/jobs/scan')); + await sourceRow(page, 'ComfyUI input').getByRole('button', { name: 'Scan folder', exact: true }).click(); + await dialog(page).getByRole('button', { name: 'Start scan', exact: true }).click(); + expect((await started).postDataJSON()).toEqual({ scope: 'new', roots: [0], paths: ['input'] }); + await expect(page).toHaveURL(/#\/activity\/[0-9a-f]{32}$/); +}); + +test('a save refused for one unusable source names it and marks its row', async ({ page, request }) => { + await expect(await request.post('/__test__/system/missing-source', { data: { inside: true } })).toBeOK(); + await openSources(page); + await expect(sourceRow(page, 'unplugged').locator('p')).toHaveText('output/unplugged · manual scan'); + // The drive is unplugged after the page read the list; removing another source resends the whole list. + await expect(await request.post('/__test__/system/unplug')).toBeOK(); + await sourceRow(page, 'ComfyUI input').getByRole('button', { name: 'Remove', exact: true }).click(); + const saved = page.waitForResponse(response => response.request().method() === 'POST' && response.url().endsWith(SETTINGS_URL)); + await saveButton(page).click(); + const answer = await saved; + expect(answer.status()).toBe(400); + expect(await answer.json()).toMatchObject({ code: 'invalid-source', source: 'output/unplugged', index: 1 }); + await expect(statusOf(page)).toHaveText("output/unplugged can't be used: Gallery root does not exist. Remove it or reconnect it, then save."); + await expect(sourceRow(page, 'unplugged')).toHaveAttribute('aria-invalid', 'true'); + await expect(sourceList(page).locator('.operation[aria-invalid="true"]')).toHaveCount(1); + expect((await serverJson(request, SETTINGS_URL)).sources.configured).toHaveLength(3); + // Removed, the list saves. + await sourceRow(page, 'unplugged').getByRole('button', { name: 'Remove', exact: true }).click(); + await expect(sourceList(page).locator('.operation[aria-invalid="true"]')).toHaveCount(0); + await saveButton(page).click(); + await expect(statusOf(page)).toHaveText('Saved to the server configuration.'); + expect((await serverJson(request, SETTINGS_URL)).sources.configured.map(source => source.path)).toEqual(['output']); +}); diff --git a/tests/browser/workspace_system_fixture.py b/tests/browser/workspace_system_fixture.py index fae432d4..95f32189 100644 --- a/tests/browser/workspace_system_fixture.py +++ b/tests/browser/workspace_system_fixture.py @@ -11,7 +11,10 @@ read. These per-server hooks only change what a System test needs on top: requests for ``path`` (a ``/prompt_manager/system/`` route) with the routes' 500 error envelope; - ``POST /__test__/system/missing-source`` configures one more source that - does not exist (an unmounted drive); + does not exist (an unmounted drive); with ``{"inside": true}`` it is an + existing folder inside the output folder (``output/unplugged``), which + ``POST /__test__/system/unplug`` then removes (a drive unplugged while the + page shows it); - ``POST /__test__/system/missing-file`` deletes the newest linked image's file from disk, leaving its database row (a missing-file link); - ``POST /__test__/system/unthumbnailed`` ``{count}`` writes ``count`` plain @@ -74,8 +77,12 @@ def register_system_commands(app, routes, api, root: Path): app.middlewares.append(inject_failures) + unplugged = root / "output" / "unplugged" + def reset(): failures.clear() + if unplugged.is_dir(): + unplugged.rmdir() GalleryConfig.MONITORING_DIRECTORIES = list(configured) api._health_report = None ( @@ -115,14 +122,26 @@ def register_system_commands(app, routes, api, root: Path): @routes.post("/__test__/system/missing-source") async def missing_source(request): - absent = str(root / "unmounted-drive") - if os.path.exists(absent): - raise web.HTTPConflict() + data = await request.json() if request.can_read_body else {} + if data.get("inside") is True: + unplugged.mkdir(exist_ok=True) + absent = str(unplugged) + else: + absent = str(root / "unmounted-drive") + if os.path.exists(absent): + raise web.HTTPConflict() GalleryConfig.MONITORING_DIRECTORIES = [*configured, absent] return web.json_response( {"success": True, "configured": len(GalleryConfig.MONITORING_DIRECTORIES)} ) + @routes.post("/__test__/system/unplug") + async def unplug(request): + if not unplugged.is_dir(): + raise web.HTTPConflict() + unplugged.rmdir() + return web.json_response({"success": True}) + @routes.post("/__test__/system/missing-file") async def missing_file(request): with api.db.model.get_connection() as conn: diff --git a/tests/js/api-client.test.js b/tests/js/api-client.test.js index df2fa211..e22878f6 100644 --- a/tests/js/api-client.test.js +++ b/tests/js/api-client.test.js @@ -355,3 +355,20 @@ test("keepalive is forwarded only when asked, so a request can outlive its page" for (const call of fetch.calls) call.resolve(jsonResponse(200, { success: true })); await Promise.all(writes); }); + +test("ApiError keeps a validated running job on busy and a validated source on invalid-source", () => { + const body = { running: { id: "a".repeat(32), title: "Scan for metadata" }, source: "output/gone" }; + const busy = new ApiError(409, "busy", "busy", null, body); + assert.deepEqual([busy.running, busy.source], [{ id: "a".repeat(32), title: "Scan for metadata" }, null]); + assert.notEqual(busy.running, body.running, "server objects must be copied"); + const invalid = new ApiError(400, "refused", "invalid-source", null, body); + assert.deepEqual([invalid.running, invalid.source], [null, "output/gone"]); + for (const running of [null, "x", { id: 1, title: "t" }, { id: "a b", title: "t" }, { id: "a", title: "" }, + { id: "a", title: "x".repeat(201) }, { id: "a".repeat(65), title: "t" }]) { + assert.equal(new ApiError(409, "busy", "busy", null, { running }).running, null); + } + for (const source of [null, 7, "", "x".repeat(4097)]) { + assert.equal(new ApiError(400, "refused", "invalid-source", null, { source }).source, null); + } + assert.deepEqual([new ApiError(409, "old caller").running, new ApiError(409, "old caller").source], [null, null]); +}); diff --git a/tests/js/workspace-system-sources.test.js b/tests/js/workspace-system-sources.test.js index 2485d60f..533241f6 100644 --- a/tests/js/workspace-system-sources.test.js +++ b/tests/js/workspace-system-sources.test.js @@ -24,14 +24,17 @@ test('each effective source is the concept row: label, "<path> · watching|manua const { rows, note } = sourceRows({ sources: STATUS, saved, current: saved }); assert.equal(note, null); assert.deepEqual(rows, [ - { key: 'output', title: 'ComfyUI output', description: 'output · watching', scan: { label: 'Scan changes', root: 0 }, remove: 'output' }, - { key: 'input', title: 'ComfyUI input', description: 'input · manual scan', scan: { label: 'Scan folder', root: 1 }, remove: 'input' }, + { key: 'output', title: 'ComfyUI output', description: 'output · watching', scan: { label: 'Scan changes', root: 0, path: 'output' }, + remove: 'output' }, + { key: 'input', title: 'ComfyUI input', description: 'input · manual scan', scan: { label: 'Scan folder', root: 1, path: 'input' }, + remove: 'input' }, ]); }); test('watching only with a live observer; restart pending otherwise (D98)', async () => { const { sourceRows, WATCH_COPY } = await sourcesModule; - assert.deepEqual(WATCH_COPY, { watching: 'watching', manual: 'manual scan', restart_pending: 'restart pending' }); + assert.deepEqual(WATCH_COPY, { watching: 'watching', stopping: 'watching until restart', manual: 'manual scan', + restart_pending: 'restart pending' }); const sources = { ...STATUS, effective: STATUS.effective.map(source => ({ ...source, watch: 'restart_pending' })) }; const { rows } = sourceRows({ sources, saved: ['output', 'input'], current: ['output', 'input'] }); assert.deepEqual(rows.map(row => [row.description, row.scan.label]), @@ -73,7 +76,8 @@ test('auto-detected sources are listed without Remove, with the line that says h const { sourceRows, NOTES } = await sourcesModule; const auto = { configured: [], effective: [{ label: 'ComfyUI output', path: 'output', root_index: 0, watch: 'manual' }], auto_detected: true }; assert.deepEqual(sourceRows({ sources: auto, saved: [], current: [] }), { rows: [ - { key: 'auto:0', title: 'ComfyUI output', description: 'output · manual scan', scan: { label: 'Scan folder', root: 0 }, remove: null }, + { key: 'auto:0', title: 'ComfyUI output', description: 'output · manual scan', scan: { label: 'Scan folder', root: 0, path: 'output' }, + remove: null }, ], note: NOTES.autoDetected }); // Every configured source unusable: ComfyUI's output folder is used instead. const fallback = { ...auto, configured: [{ label: 'gone', path: 'gone', valid: false, code: 'not-found', reason: 'Gallery root does not exist' }] }; @@ -121,3 +125,21 @@ test('Refresh files toasts the rescan counts, or says it finished when they are assert.equal(refreshedText({ total: -1, roots: 1 }), 'Files refreshed.'); assert.equal(refreshedText(null), 'Files refreshed.'); }); + +test('monitoring switched off: a source still watched says so until the restart (D-NEW-S4-2)', async () => { + const { sourceRows } = await sourcesModule; + const sources = { ...STATUS, effective: [{ ...STATUS.effective[0], watch: 'stopping' }, STATUS.effective[1]] }; + const { rows } = sourceRows({ sources, saved: ['output', 'input'], current: ['output', 'input'] }); + assert.deepEqual(rows.map(row => [row.description, row.scan.label]), + [['output · watching until restart', 'Scan changes'], ['input · manual scan', 'Scan folder']]); +}); + +test('a valid configured source missing from the effective list is another spelling of a listed folder (D-NEW-S4-2)', async () => { + const { sourceRows, DUPLICATE_SOURCE } = await sourcesModule; + assert.equal(DUPLICATE_SOURCE, 'same folder as another source'); + const sources = { ...STATUS, configured: [...STATUS.configured, + { label: 'output', path: 'output/.', valid: true, code: null, reason: null }] }; + const saved = ['output', 'input', 'output/.']; + assert.deepEqual(sourceRows({ sources, saved, current: saved }).rows.at(-1), { key: 'output/.', title: 'output', + description: 'output/. · same folder as another source', scan: null, remove: 'output/.' }); +}); diff --git a/tests/js/workspace-system-state.test.js b/tests/js/workspace-system-state.test.js index af60efb0..05b39032 100644 --- a/tests/js/workspace-system-state.test.js +++ b/tests/js/workspace-system-state.test.js @@ -192,6 +192,7 @@ test('status text per outcome', async () => { partial: 'Some changes were not saved. Review the highlighted settings.', tabOnly: 'Saved for this tab only; browser storage is unavailable.', conflict: 'Settings changed elsewhere. Reload this section to see them.', + restart: 'Restart ComfyUI to apply:', }); const clean = { browser: 'clean', host: 'clean', runtime: 'clean' }; assert.equal(statusText(clean), 'No unsaved changes.'); @@ -316,3 +317,50 @@ test('host read state says whether the server settings could be read', async () assert.equal(await drafts.reloadHost(), true); assert.equal(drafts.hostState(), 'loaded'); }); + +test('a host save names the settings that apply after a restart (D-NEW-S4-2)', async () => { + const { statusText, RESTART_LABELS } = await source; + assert.deepEqual(RESTART_LABELS, { monitoring_enabled: 'Watch for new outputs' }); + const saved = { browser: 'clean', host: 'saved', runtime: 'clean' }; + assert.equal(statusText({ ...saved, restart: ['monitoring_enabled'] }), + 'Saved to the server configuration. Restart ComfyUI to apply: Watch for new outputs.'); + // A field without a row label (the sources say "restart pending" per row) adds nothing. + assert.equal(statusText({ ...saved, restart: ['gallery_root_paths'] }), 'Saved to the server configuration.'); + assert.equal(statusText({ ...saved, restart: [] }), 'Saved to the server configuration.'); + + const domains = adapters({ host: { monitoring_enabled: true } }); + const drafts = await loaded(domains); + domains.outcome.host = () => ({ success: true, persisted: true, revision: 'sha256:two', + restart_required_fields: ['monitoring_enabled', 7] }); + drafts.set('host', 'monitoring_enabled', false); + assert.deepEqual(await drafts.save(), { browser: 'clean', host: 'saved', runtime: 'clean', restart: ['monitoring_enabled'] }); + assert.equal(drafts.statusText(), 'Saved to the server configuration. Restart ComfyUI to apply: Watch for new outputs.'); + // The next edit is unsaved again. + drafts.set('host', 'monitoring_enabled', true); + assert.equal(drafts.statusText(), 'Unsaved changes.'); +}); + +test('a host save refused for one source says the server reason and names it until the next edit', async () => { + const domains = adapters({ host: { gallery_root_paths: ['output', 'output/gone'] } }); + const drafts = await loaded(domains); + const message = "output/gone can't be used: Gallery root does not exist. Remove it or reconnect it, then save."; + domains.outcome.host = () => { throw Object.assign(new Error(message), { code: 'invalid-source', source: 'output/gone' }); }; + drafts.set('host', 'gallery_root_paths', ['output/gone']); + const outcome = await drafts.save(); + assert.deepEqual(outcome, { browser: 'clean', host: 'failed', runtime: 'clean', + invalid: { source: 'output/gone', message } }); + assert.equal(drafts.statusText(), message); + assert.equal(drafts.invalidSource(), 'output/gone'); + assert.deepEqual([...drafts.failed('host')], ['gallery_root_paths']); + // Any other failure keeps the generic copy and names no source. + domains.outcome.host = () => { throw Object.assign(new Error('busy'), { code: 'busy' }); }; + await drafts.save(); + assert.equal(drafts.statusText(), 'Some changes were not saved. Review the highlighted settings.'); + assert.equal(drafts.invalidSource(), null); + domains.outcome.host = () => { throw Object.assign(new Error(message), { code: 'invalid-source' }); }; + await drafts.save(); + assert.equal(drafts.invalidSource(), null); + drafts.set('host', 'gallery_root_paths', ['output']); + assert.equal(drafts.invalidSource(), null); + assert.equal(drafts.statusText(), 'Unsaved changes.'); +}); diff --git a/tools/vendor/frontend-coverage-baseline.json b/tools/vendor/frontend-coverage-baseline.json index 75061d57..75926eb5 100644 --- a/tools/vendor/frontend-coverage-baseline.json +++ b/tools/vendor/frontend-coverage-baseline.json @@ -7,7 +7,7 @@ }, "api-client.js": { "lines": 98, - "branches": 95, + "branches": 96, "functions": 98, "statements": 98 }, @@ -470,7 +470,7 @@ "workspace/system-sources.mjs": { "lines": 98, "branches": 88, - "functions": 94, + "functions": 95, "statements": 98 }, "workspace/system-state.mjs": { diff --git a/web/js/api-client.js b/web/js/api-client.js index b3344677..ee21d365 100644 --- a/web/js/api-client.js +++ b/web/js/api-client.js @@ -40,6 +40,21 @@ Number.isSafeInteger(current.tag_revision) && current.tag_revision >= 0) { this.current = { tags: current.tags.slice(), tag_revision: current.tag_revision }; } + // A busy answer names the running job that holds the slot; an + // invalid-source answer names the source it refused. Only + // well-formed values are kept. + this.running = null; + const running = current && current.running; + if (this.code === "busy" && running && typeof running.id === "string" && + /^[A-Za-z0-9_-]{1,64}$/.test(running.id) && typeof running.title === "string" && + running.title.length > 0 && running.title.length <= 200) { + this.running = { id: running.id, title: running.title }; + } + this.source = null; + if (this.code === "invalid-source" && current && typeof current.source === "string" && + current.source.length > 0 && current.source.length <= 4096) { + this.source = current.source; + } } } diff --git a/web/js/workspace/activity-scan.mjs b/web/js/workspace/activity-scan.mjs index 9c86f99a..f92af7bd 100644 --- a/web/js/workspace/activity-scan.mjs +++ b/web/js/workspace/activity-scan.mjs @@ -17,6 +17,8 @@ export const REPORT_DONE = 'Job report downloaded.'; export const REPORT_FAILED = 'The report could not be downloaded. Try again.'; const JOBS_REPORT_BASE = '/prompt_manager/jobs'; export const START_FAILED = 'The scan could not start. Try again.'; +// A per-source scan whose sources changed since they were read (S4): the server's 409 says so. +const SOURCES_CHANGED_CODE = 'sources-changed'; const FALLBACK_NAME = 'prompts_backup.db'; const REPORT_FALLBACK_NAME = 'prompt-manager-report.json'; @@ -81,29 +83,37 @@ export function busyToast(running) { /** * Start the scan job over `files` ('new': new and unresolved files, or 'all') and open its view; a refused or failed - * start says why and stays on the page. `roots` (System › Sources, S4) limits it to those effective source indexes. + * start says why and stays on the page. `roots` (System › Sources, S4) limits it to those effective source indexes, and + * `paths` names the public path read for each: when the sources changed since (409 `sources-changed`) nothing starts, + * the toast says the server's reason and `onStale` (which every caller with `roots` passes) reads them again. */ -export async function startMetadataScan({ store, scope, files = 'new', roots = null, notify = showWorkspaceToast, - navigate = navigateTo }) { +export async function startMetadataScan({ store, scope, files = 'new', roots = null, paths = null, onStale, + notify = showWorkspaceToast, navigate = navigateTo }) { try { - const result = await store.start('scan', roots ? { scope: files, roots } : { scope: files }); + const result = await store.start('scan', roots ? { scope: files, roots, paths } : { scope: files }); if (scope.signal.aborted) return; if (result.busy !== undefined) { notify(busyToast(result.busy)); return; } navigate(`#/activity/${result.job.id}`); - } catch { - if (!scope.signal.aborted) notify(START_FAILED); + } catch (error) { + if (scope.signal.aborted) return; + if (error?.code === SOURCES_CHANGED_CODE) { + notify(error.message); + onStale(); + return; + } + notify(START_FAILED); } } /** - * Open "Review metadata scan" for `sources` output folders; with `roots` (one source's index, System › Sources) the - * scan reads only those. + * Open "Review metadata scan" for `sources` output folders; with `roots` and `paths` (one source's index and public + * path, System › Sources) the scan reads only those, and `onStale` runs when they changed since they were read. */ -export function openMetadataScan({ scope, store, sources, roots = null, trigger = null }) { +export function openMetadataScan({ scope, store, sources, roots = null, paths = null, onStale, trigger = null }) { openScanReviewDialog({ scope, sources, trigger, onBackup: () => downloadDatabaseBackup(), - onStart: ({ scope: files }) => startMetadataScan({ store, scope, files, roots }) }); + onStart: ({ scope: files }) => startMetadataScan({ store, scope, files, roots, paths, onStale }) }); } diff --git a/web/js/workspace/system-sources.mjs b/web/js/workspace/system-sources.mjs index c42996d7..5403409d 100644 --- a/web/js/workspace/system-sources.mjs +++ b/web/js/workspace/system-sources.mjs @@ -5,9 +5,11 @@ * update notice (`pm_update_notice_dismissed`, the legacy admin page's own key). Later dismisses it without scanning; * Review scan dismisses it, as the legacy page's Scan does, and reviews every source. * - "Image sources" lists the server's sources (GET /settings, S4 Task 9) in `#source-list`, one `.operation` each: - * its label, "<path> · watching | manual scan | restart pending" and "Scan changes" (watched) or "Scan folder", which - * opens the shared "Review metadata scan" (D55) for that one source ("1 output folder") and starts the session job over - * it (`roots`, S4 Task 10). The job opens in Activity; the toast only says it started. + * its label, "<path> · watching | watching until restart | manual scan | restart pending" and "Scan changes" + * (watched) or "Scan folder", which opens the shared "Review metadata scan" (D55) for that one source ("1 output + * folder") and starts the session job over it (`roots` with its public path, S4 Task 10). The job opens in Activity; + * the toast only says it started. When the sources changed since they were read, nothing starts: the toast says so + * and the list is read again. * - "+ Add source" asks the server whether a folder can be a source (`validate-source`, which writes nothing), then adds * it to the host draft of `gallery_root_paths`; Remove takes one out. Both wait for the one Save row. * - "Watch for new outputs" (`monitoring_enabled`) and "Worker threads" (`worker_threads`, a range up to the server's @@ -17,8 +19,11 @@ * * D98: per-source Remove; an added source reads "not saved yet" until Save; the line under the list says when * ComfyUI's output folder is used instead; "restart pending" for a source the running monitor does not watch yet; the - * "Refresh files" operation (`/gallery/rescan`) closes the surface. D-NEW-S4-1: the add dialog's field is "Folder - * path", starts empty and says the server's reason in place; its action is "Add". + * "Refresh files" operation (`/gallery/rescan`) closes the surface. D127: the add dialog's field is "Folder path", + * starts empty and says the server's reason in place; its action is "Add", and Enter adds too. + * D-NEW-S4-2: "watching until restart" for a source the running monitor still watches after monitoring was switched off; + * "same folder as another source" for a configured source the effective list already holds under another spelling; a + * save refused for one source (`invalid-source`) marks that row and the status says the server's reason. */ import { createButton } from '../shared/toolkit/core.js'; import { closeWorkspaceDialog, field, openContentDialog } from './workspace-dialogs.mjs'; @@ -36,8 +41,12 @@ const ROOTS_KEY = 'gallery_root_paths'; export const NOTICE_KEY = 'pm_update_notice_dismissed'; export const NOTICE_VERSION = '3.2.0'; -export const WATCH_COPY = Object.freeze({ watching: 'watching', manual: 'manual scan', restart_pending: 'restart pending' }); +export const WATCH_COPY = Object.freeze({ watching: 'watching', stopping: 'watching until restart', manual: 'manual scan', + restart_pending: 'restart pending' }); +// The states whose observer runs now: their scan only has to catch up on changes. +const WATCHED = new Set(['watching', 'stopping']); const NOT_SAVED = 'not saved yet'; +export const DUPLICATE_SOURCE = 'same folder as another source'; /** D98: the line under the source list when it does not say everything by itself. */ export const NOTES = Object.freeze({ removedAll: 'No source is configured. After saving, ComfyUI’s output folder is used.', @@ -85,15 +94,16 @@ export function draftRoots(next, saved) { const folderName = path => path.split('/').filter(Boolean).at(-1) ?? path; function effectiveRow(source, remove) { - const watched = source.watch === 'watching'; + const watched = WATCHED.has(source.watch); return { key: remove ?? `auto:${source.root_index}`, title: source.label, description: `${source.path} · ${WATCH_COPY[source.watch] ?? WATCH_COPY.manual}`, - scan: { label: watched ? 'Scan changes' : 'Scan folder', root: source.root_index }, remove }; + scan: { label: watched ? 'Scan changes' : 'Scan folder', root: source.root_index, path: source.path }, remove }; } /** * The source list for `sources` (GET /settings' `sources`), the saved `gallery_root_paths` and the drafted ones - * (`current`): `{rows: [{key, title, description, scan: {label, root} | null, remove: path | null}], note}`. + * (`current`): `{rows: [{key, title, description, scan: {label, root, path} | null, remove: path | null}], note}`. + * A valid configured source missing from the effective list is another spelling of a folder already listed. */ export function sourceRows({ sources, saved, current, labels = addedLabels }) { const configured = sources.configured ?? []; @@ -107,7 +117,8 @@ export function sourceRows({ sources, saved, current, labels = addedLabels }) { const live = entry?.valid ? effective.find(source => source.path === path) : undefined; if (live) rows.push(effectiveRow(live, path)); else if (entry) { - rows.push({ key: path, title: entry.label, description: `${path} · ${entry.reason ?? WATCH_COPY.manual}`, scan: null, remove: path }); + const why = entry.valid ? DUPLICATE_SOURCE : entry.reason ?? WATCH_COPY.manual; + rows.push({ key: path, title: entry.label, description: `${path} · ${why}`, scan: null, remove: path }); } else rows.push({ key: path, title: labels.get(path) ?? folderName(path), description: `${path} · ${NOT_SAVED}`, scan: null, remove: path }); } if (dirty) return { rows, note: roots.length ? null : NOTES.removedAll }; @@ -149,8 +160,8 @@ function noticeNode({ scope, onLater, onReview }) { } /** - * "Add image source" (the concept's `simpleInput`, D-NEW-S4-1): the server checks the folder; a refused one keeps the - * dialog open with the server's reason under the field. + * "Add image source" (the concept's `simpleInput`, D127): the server checks the folder; a refused one keeps the + * dialog open with the server's reason under the field. Enter in the field adds, as the Add button does. */ function openAddSource({ api, scope, trigger, listed, onAdd }) { const input = el('input'); @@ -165,12 +176,18 @@ function openAddSource({ api, scope, trigger, listed, onAdd }) { input.setAttribute('aria-invalid', 'true'); input.focus(); }; - input.addEventListener('input', () => { + scope.listen(input, 'input', () => { reason.remove(); input.removeAttribute('aria-invalid'); }); - cancel.addEventListener('click', () => closeWorkspaceDialog()); - add.addEventListener('click', async () => { + scope.listen(input, 'keydown', event => { + if (event.key !== 'Enter' || event.isComposing) return; + event.preventDefault(); + submit(); + }); + scope.listen(cancel, 'click', () => closeWorkspaceDialog()); + scope.listen(add, 'click', () => submit()); + async function submit() { const path = input.value.trim(); if (!path) { input.focus(); return; } if (add.getAttribute('aria-disabled') === 'true') return; @@ -189,7 +206,7 @@ function openAddSource({ api, scope, trigger, listed, onAdd }) { } finally { add.removeAttribute('aria-disabled'); } - }); + } const actions = el('div', undefined, 'dialog-actions'); actions.append(cancel, add); openContentDialog({ scope, title: 'Add image source', trigger, nodes: [field('Folder path', input, 'action-input'), actions], @@ -278,7 +295,8 @@ export function mountSources(container, { api, activity, scope, drafts, notify = const actions = []; if (row.scan) { const scan = createButton(row.scan.label); - scope.listen(scan, 'click', () => openMetadataScan({ scope, store: activity, trigger: scan, sources: 1, roots: [row.scan.root] })); + scope.listen(scan, 'click', () => openMetadataScan({ scope, store: activity, trigger: scan, sources: 1, + roots: [row.scan.root], paths: [row.scan.path], onStale: reread })); actions.push(scan); } if (row.remove !== null) { @@ -288,6 +306,8 @@ export function mountSources(container, { api, activity, scope, drafts, notify = actions.push(remove); } const node = el('div', undefined, 'operation'); + // D-NEW-S4-2: the source the last save was refused for; the status says why. + if (row.remove !== null && row.remove === drafts.invalidSource?.()) node.setAttribute('aria-invalid', 'true'); const text = el('div'); text.append(el('h3', row.title), el('p', row.description)); const group = el('div', undefined, 'row-actions'); @@ -335,6 +355,12 @@ export function mountSources(container, { api, activity, scope, drafts, notify = } } + // The sources changed since this page read them (a refused per-source scan): read them and the saved values again. + function reread() { + load(); + drafts.reloadHost?.(); + } + async function load() { sources = undefined; paintList(); diff --git a/web/js/workspace/system-state.mjs b/web/js/workspace/system-state.mjs index b827dc3e..cdedba40 100644 --- a/web/js/workspace/system-state.mjs +++ b/web/js/workspace/system-state.mjs @@ -17,8 +17,10 @@ * Adapters: `readBrowser()` and `readRuntime()` return the values; `readHost()` returns `{values, revision}`. * `writeBrowser(changes)` returns `{persisted}` (false: kept for this tab only) and throws an error with `code: * 'settings-changed'` when another page changed a drafted value (a conflict: the browser values are read again), - * `writeHost(changes, {expected_revision})` the settings response (`{success, persisted, revision}`), `writeRuntime(changes)` anything; - * a write that throws failed. + * `writeHost(changes, {expected_revision})` the settings response (`{success, persisted, revision}`, and the + * `restart_required_fields` the status names), `writeRuntime(changes)` anything; a write that throws failed. A host + * write refused as `invalid-source` (its error's `source` and server message) makes that message the status and + * `invalidSource()` that source, so Sources marks its row (S4). */ export { SYSTEM_SECTIONS, isSystemSection } from './state.mjs'; @@ -32,21 +34,35 @@ export const SETTINGS_STATUS = Object.freeze({ partial: 'Some changes were not saved. Review the highlighted settings.', tabOnly: 'Saved for this tab only; browser storage is unavailable.', conflict: 'Settings changed elsewhere. Reload this section to see them.', + // D-NEW-S4-2: a saved setting the running server applies only after a restart. + restart: 'Restart ComfyUI to apply:', }); +/** D-NEW-S4-2: the settings a host save can report in `restart_required_fields`, by their row label. */ +export const RESTART_LABELS = Object.freeze({ monitoring_enabled: 'Watch for new outputs' }); + export const DRAFT_DOMAINS = Object.freeze(['browser', 'host', 'runtime']); const CONFLICT_CODE = 'settings-changed'; +const INVALID_SOURCE_CODE = 'invalid-source'; const ADAPTERS = ['readBrowser', 'readHost', 'readRuntime', 'writeBrowser', 'writeHost', 'writeRuntime']; -/** The status copy of one `save()` outcome (`{browser, host, runtime}`, each clean/saved/tab-only/applied/failed/conflict). */ +/** + * The status copy of one `save()` outcome (`{browser, host, runtime}`, each clean/saved/tab-only/applied/failed/conflict). + * `restart` lists the host fields the server applies at its next start, `invalid` the source a refused host save named + * (`{source, message}`): its message is the status (D-NEW-S4-2). + */ export function statusText(outcome) { const results = DRAFT_DOMAINS.map(domain => outcome?.[domain]); if (results.includes('conflict')) return SETTINGS_STATUS.conflict; - if (results.includes('failed')) return SETTINGS_STATUS.partial; + if (results.includes('failed')) return outcome?.invalid?.message || SETTINGS_STATUS.partial; const saved = []; if (outcome?.browser === 'saved') saved.push(SETTINGS_STATUS.browser); if (outcome?.browser === 'tab-only') saved.push(SETTINGS_STATUS.tabOnly); - if (outcome?.host === 'saved') saved.push(SETTINGS_STATUS.host); + if (outcome?.host === 'saved') { + saved.push(SETTINGS_STATUS.host); + const restart = (outcome.restart ?? []).map(field => RESTART_LABELS[field]).filter(Boolean); + if (restart.length) saved.push(`${SETTINGS_STATUS.restart} ${restart.join(', ')}.`); + } if (outcome?.runtime === 'applied') saved.push(SETTINGS_STATUS.runtime); return saved.join(' ') || SETTINGS_STATUS.clean; } @@ -68,6 +84,8 @@ export function createSystemDrafts(adapters) { let hostRead = 'pending'; let outcome = null; let saving = null; + // What the last host save said beyond its result: the restart fields, or the source it refused. + let hostDetail = {}; const notify = () => { for (const listener of [...listeners]) listener(); }; const changes = domain => Object.fromEntries(drafts[domain]); @@ -150,6 +168,8 @@ export function createSystemDrafts(adapters) { if (result?.code === CONFLICT_CODE) { fail(domain, written); return 'conflict'; } if (!result?.success || result.persisted !== true) { fail(domain, written); return 'failed'; } if (typeof result.revision === 'string') revision = result.revision; + const restart = result.restart_required_fields; + if (Array.isArray(restart)) hostDetail.restart = restart.filter(field => typeof field === 'string'); commit(domain, written); return 'saved'; } @@ -158,6 +178,9 @@ export function createSystemDrafts(adapters) { return 'applied'; } catch (error) { fail(domain, written); + if (domain === 'host' && error?.code === INVALID_SOURCE_CODE && typeof error.message === 'string') { + hostDetail.invalid = { source: typeof error.source === 'string' ? error.source : null, message: error.message }; + } return error?.code === CONFLICT_CODE ? 'conflict' : 'failed'; } } @@ -172,8 +195,10 @@ export function createSystemDrafts(adapters) { function save() { saving ??= (async () => { for (const domain of DRAFT_DOMAINS) failedKeys[domain].clear(); + hostDetail = {}; const result = {}; for (const domain of DRAFT_DOMAINS) result[domain] = await saveDomain(domain); + Object.assign(result, hostDetail); // Settings changed elsewhere: read them (and their revision), keeping the drafts (Q10). if (result.host === 'conflict') await reloadHost(); if (result.browser === 'conflict') await load('browser').catch(() => {}); @@ -228,6 +253,8 @@ export function createSystemDrafts(adapters) { changes, saving: () => saving !== null, failed: domain => { requireDomain(domain); return new Set(failedKeys[domain]); }, + /** The source the last save was refused for (its public path), until the next edit; else null. */ + invalidSource: () => outcome?.invalid?.source ?? null, /** The Save row's text: the last save's outcome until the next edit, else unsaved or clean. */ statusText: () => (outcome ? statusText(outcome) : dirty() ? SETTINGS_STATUS.dirty : SETTINGS_STATUS.clean), subscribe(listener) { diff --git a/web/js/workspace/system.mjs b/web/js/workspace/system.mjs index 18a1ae23..1b4b8705 100644 --- a/web/js/workspace/system.mjs +++ b/web/js/workspace/system.mjs @@ -39,11 +39,12 @@ export const SECTION_HANDOFFS = Object.freeze({ export const NOT_DELIVERED = 'This setting is not available here yet.'; let drafts = null; -// The Activity store of the System page shown last: a refused host save names the running job from its list. +// The Activity store of the System page shown last: a refused host save that does not name its job reads the list. let jobs = null; -/** The running job that holds the registry, from a fresh list read (the store's load never throws). */ -async function runningJob() { +/** The running job a 409 `busy` named, else the first running one from a fresh list read (the load never throws). */ +async function runningJob(error) { + if (error?.running) return error.running; await jobs.load(); return jobs.running()[0]; } @@ -86,7 +87,7 @@ export function systemDrafts(api) { return await api.post(SETTINGS_URL, { ...changes, expected_revision }); } catch (error) { // A sources change waits for a running record-changing job (S4): name it, as a refused start does (D57). - if (error?.code === 'busy') showWorkspaceToast(busyToast(await runningJob())); + if (error?.code === 'busy') showWorkspaceToast(busyToast(await runningJob(error))); throw error; } }, diff --git a/web/js/workspace/workspace.css b/web/js/workspace/workspace.css index ace96b00..2de1f883 100644 --- a/web/js/workspace/workspace.css +++ b/web/js/workspace/workspace.css @@ -89,6 +89,8 @@ main:focus-visible { outline: 2px solid var(--ca-focus); outline-offset: 4px; } not save is highlighted, as its "Review the highlighted settings" status says. */ .save-row button[aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* D92 */ } .setting-row [aria-invalid="true"] { outline: 2px solid var(--red); outline-offset: 2px; /* D92 */ } +/* D92: a source the last save was refused for (System › Sources) is highlighted the same way. */ +#source-list > .operation[aria-invalid="true"] { outline: 2px solid var(--red); outline-offset: 2px; /* D92 */ } /* D93: an Overview row whose action is not available yet (the review queue, Q6) keeps its button, disabled. */ .operation .row-actions button[aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* D93 */ } /* D97: Appearance's legacy display rows. "Show image details on cards" off keeps a card's Info and Tag at the end of