diff --git a/TESTING.md b/TESTING.md index dbbfb38b..7c457309 100644 --- a/TESTING.md +++ b/TESTING.md @@ -3180,21 +3180,6 @@ the per-section hand-off panel, was retired by S12 (#329): every section is nati `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. -- **D147** (Sources, #247 D-Q4): the concept has no switch for prompts from workflows - without a PromptManager node; the maintainer chose a visible switch over a config key. - A `.setting-row` in the concept's toggle markup follows "Watch for new outputs": - "Save prompts from other workflows" (`#foreign-runs`, host key `save_foreign_runs`, - config.json `gallery.monitoring.save_foreign_runs`, default on), "New outputs of - workflows without a PromptManager node add their prompt to the library when it is - certain.". It is saved with the revision like the other host rows and applied at once - (the monitor reads it for every file), so a save names no restart. While "Watch for - new outputs" is drafted off the switch is disabled and a `p.small` under the - description says "Applies while Watch for new outputs is on." (the value is kept). - Pinned by `sources matches the concept` (the mapping inserts the row after the - monitoring row), `sources comparison detects mutations` (a missing row and a changed - label are caught), `D147: Save prompts from other workflows saves save_foreign_runs - live; off while monitoring is off (#247)` (workspace-system-sources.spec: keyboard on - desktop, a tap on touch) and the system-sources node test. - **D129** (library location, S6b): the concept shows neither where the library lives nor a way to move it. System › Overview's "Library health" gains a "Library location" `.operation` row after Backup status, in that row's own markup: " · ", @@ -3304,6 +3289,26 @@ the per-section hand-off panel, was retired by S12 (#329): every section is nati Cancel-only review matches the concept review shell` (system-tagging-concept.spec) and the batch tests of workspace-system-tagging.spec; the mutation check renames each grid label in turn. +- **D147** (Sources, #247 D-Q4): the concept has no switch for prompts from workflows + without a PromptManager node; the maintainer chose a visible switch over a config key. + A `.setting-row` in the concept's toggle markup follows "Watch for new outputs": + "Save prompts from other workflows" (`#foreign-runs`, host key `save_foreign_runs`, + config.json `gallery.monitoring.save_foreign_runs`, default on), "New outputs of + workflows without a PromptManager node add their prompt to the library when it is + certain.". It is saved with the revision like the other host rows and applied at once + (the monitor reads it for every file), so a save names no restart. While "Watch for + new outputs" is drafted off, a `p.small` under the description says "Applies while + Watch for new outputs is on." (the switch's `aria-describedby`; the value is kept) and + the switch is disabled, except while a source still reads "watching until restart" + (D128): the running observer still delivers files then, so the value can be set. The + server stores no foreign prompt while `monitoring_enabled` is saved off, whatever the + observer does. + Pinned by `sources matches the concept` (the mapping inserts the row after the + monitoring row), `sources comparison detects mutations` (a missing row and a changed + label are caught), `D147: Save prompts from other workflows saves save_foreign_runs + live; off while monitoring is off (#247)` (keyboard on desktop, a tap on touch) and + `D147: saved off while the monitor still watches until a restart, the foreign row can + still be set (D128)` (workspace-system-sources.spec) and the system-sources node test. - **D109** (Logs, S12, #329): the concept's Logs surface on the live server. The toolbar keeps "Level", Refresh and the download (renamed "Download log": the active log, `/logs/download/prompt_manager.log`, through the api client), with all six minimum @@ -5115,8 +5120,12 @@ handler's `_ingest_foreign`, `_recheck_foreign`, `_store_foreign`): prompt is stored and the file linked at once. `history` (an undecided graph inside ComfyUI's output folder): the file is registered in its own waiting set, then the ladder runs again at once with the file's recorded output, because the run may have - been sealed before the file settled; certain then means stored, otherwise it keeps - waiting. Anything else is logged "No certain prompt for ; left unlinked". + been sealed before the file settled. When that lookup finds a record, its answer is + final: certain is stored, anything else is logged "No certain prompt for ; + left unlinked" and the file stops waiting (no seal is coming). When it finds none, + only the inspector's or the recorded saver's answer is stored; anything else (the + legacy floor included) keeps waiting for the re-check. Anything else at the first + pass is logged "No certain prompt for ; left unlinked". - **After the run ends.** When the run's outputs are sealed, each waiting foreign file gets one re-check (`ForeignRecheck(reference)`) with the sealed output reference, so history is asked with the spelling it recorded (a Windows folder whose case @@ -5125,8 +5134,10 @@ handler's `_ingest_foreign`, `_recheck_foreign`, `_store_foreign`): the legacy floor in the monitor (D-Q3); an untraceable one still is. - **Waiting sets.** `_awaiting_foreign` and `_foreign_due` sit beside the keyless sets with the same bounds (1000 files, one hour) but are separate, so a large foreign run - never evicts a waiting keyless file. An expired or evicted entry is logged "No - recorded run for ; left unlinked". `awaiting_foreign()` lists them; + never evicts a waiting keyless file. An expired entry is logged "No recorded run + for ; left unlinked", an evicted one (waiting or due) "Too many files wait for + their run; left unlinked", both after the lock is released. A due re-check + is dropped only when it was scheduled, never a newer reference for the same file. `awaiting_foreign()` lists them; `shutdown()` and `close()` clear them, and a closed handler re-checks nothing. - **Store.** One write through `survive_switch` (rerun after a Move library): a pair through `ingest_resolved_pair(..., marks=RUN_MARKS)`, a positive through @@ -5139,8 +5150,10 @@ handler's `_ingest_foreign`, `_recheck_foreign`, `_store_foreign`): stays 0, no `last_used_at`), and nothing is queued for Needs review. - **The setting.** `GalleryConfig.SAVE_FOREIGN_RUNS` (`gallery.monitoring. save_foreign_runs`, default true, only a bool applies) is read for every file, at - settle and at the re-check; off logs "No PromptManager prompt in workflow, skipping - image" as before. System › Sources & scans "Save prompts from other workflows" (D147) + settle, at the re-check and again just before the store, together with + `MONITORING_ENABLED`: either saved off stops foreign writes at once (the running + observer watches until a restart, D128); off logs "No PromptManager prompt in + workflow, skipping image" as before. System › Sources & scans "Save prompts from other workflows" (D147) saves it through `POST /prompt_manager/settings` with no restart field; the diagnostics report lists it. @@ -5148,7 +5161,10 @@ Tests: `tests/test_run_attribution.py` `TestMonitorForeignRuns` (dataset branche linked after the seal, a seal before settle, a name two runs reported, the setting off at settle and at the re-check, a plain workflow linked at settle with no run count and the tracker queue untouched, the sealed reference's spelling, the separate -bound, expiry, a seal while the pending set is full, shutdown, a failed store), +bound, expiry, a seal while the pending set is full, shutdown, a failed store, no +record at settle keeps waiting, a record without an answer ends the wait, monitoring +saved off, the setting read before the store, a seal between registering and the +settle lookup, a newer due reference kept, evictions logged, one link per file), `tests/test_image_linking.py` `TestForeignWorkflows`, `tests/test_image_monitor_singleton.py` (close drops a foreign re-check), `tests/test_config.py`, `tests/test_settings_sources.py` `TestSaveForeignRunsSetting`, and the real-host diff --git a/tests/browser/workspace-system-sources.spec.mjs b/tests/browser/workspace-system-sources.spec.mjs index 52aba04c..b785a5b3 100644 --- a/tests/browser/workspace-system-sources.spec.mjs +++ b/tests/browser/workspace-system-sources.spec.mjs @@ -269,40 +269,68 @@ test('Watch for new outputs saves monitoring_enabled; a restart applies it', asy }); test('D147: Save prompts from other workflows saves save_foreign_runs live; off while monitoring is off (#247)', async ({ page, request }, testInfo) => { - const layout = await openSources(page); - const rows = layout.locator('section.surface > .setting-row'); - await expect(rows.locator(':scope > div > label')).toHaveText(['Watch for new outputs', 'Save prompts from other workflows', 'Worker threads']); - const foreign = layout.getByRole('switch', { name: 'Save prompts from other workflows', exact: true }); - const foreignRow = rows.filter({ has: page.locator('#foreign-runs') }); - await expect(foreignRow.locator(':scope > div > p')).toHaveText('New outputs of workflows without a PromptManager node add their prompt to the library when it is certain.'); - await expect(foreign).toBeChecked(); - await expect(foreign).toBeEnabled(); - // Keyboard on desktop, a tap on the touch project. - if (testInfo.project.name === 'touch') await foreign.tap(); - else { - await foreign.focus(); - await page.keyboard.press('Space'); + try { + const layout = await openSources(page); + const rows = layout.locator('section.surface > .setting-row'); + await expect(rows.locator(':scope > div > label')).toHaveText(['Watch for new outputs', 'Save prompts from other workflows', 'Worker threads']); + const foreign = layout.getByRole('switch', { name: 'Save prompts from other workflows', exact: true }); + const foreignRow = rows.filter({ has: page.locator('#foreign-runs') }); + await expect(foreignRow.locator(':scope > div > p')).toHaveText('New outputs of workflows without a PromptManager node add their prompt to the library when it is certain.'); + await expect(foreign).toBeChecked(); + await expect(foreign).toBeEnabled(); + // Keyboard on desktop, a tap on the touch project. + if (testInfo.project.name === 'touch') await foreign.tap(); + else { + await foreign.focus(); + await page.keyboard.press('Space'); + } + await expect(foreign).not.toBeChecked(); + await expect(statusOf(page)).toHaveText('Unsaved changes.'); + const saved = page.waitForResponse(response => response.request().method() === 'POST' && response.url().endsWith(SETTINGS_URL)); + await saveButton(page).click(); + const response = await saved; + expect(response.request().postDataJSON()).toEqual(expect.objectContaining({ save_foreign_runs: false })); + expect((await response.json()).restart_required_fields).toEqual([]); + // Applied live: the monitor reads it for every file, so no restart is named. + await expect(statusOf(page)).toHaveText('Saved to the server configuration.'); + expect((await serverJson(request, SETTINGS_URL)).settings.save_foreign_runs).toBe(false); + // With Watch for new outputs drafted off nothing is watched: the row is disabled and its note says why. + const monitoring = layout.getByRole('switch', { name: 'Watch for new outputs', exact: true }); + await monitoring.uncheck(); + await expect(foreign).toBeDisabled(); + const note = foreignRow.locator('p.small'); + await expect(note).toHaveText('Applies while Watch for new outputs is on.'); + await expect(foreign).toHaveAttribute('aria-describedby', await note.getAttribute('id')); + await monitoring.check(); + await expect(foreign).toBeEnabled(); + await expect(note).toHaveCount(0); + await expect(foreign).not.toHaveAttribute('aria-describedby', /./); + await expect(statusOf(page)).toHaveText('No unsaved changes.'); + } finally { + // The server is shared by the spec's tests: put the default back whatever happened. + await request.post(SETTINGS_URL, { data: { save_foreign_runs: true } }); + } +}); + +test('D147: saved off while the monitor still watches until a restart, the foreign row can still be set (D128)', async ({ page, request }) => { + await expect(await request.post('/__test__/system/monitor', { data: { watching: ['output', 'input'] } })).toBeOK(); + try { + const layout = await openSources(page); + await layout.getByRole('switch', { name: 'Watch for new outputs', exact: true }).uncheck(); + await saveButton(page).click(); + await expect(sourceList(page).locator('.operation p').first()).toHaveText('output · watching until restart'); + const foreign = layout.getByRole('switch', { name: 'Save prompts from other workflows', exact: true }); + await expect(foreign).toBeEnabled(); + await expect(layout.locator('.setting-row').filter({ has: page.locator('#foreign-runs') }).locator('p.small')) + .toHaveText('Applies while Watch for new outputs is on.'); + await foreign.uncheck(); + const saved = page.waitForRequest(item => item.method() === 'POST' && item.url().endsWith(SETTINGS_URL)); + await saveButton(page).click(); + expect((await saved).postDataJSON()).toEqual({ save_foreign_runs: false, expected_revision: expect.any(String) }); + await expect(statusOf(page)).toHaveText('Saved to the server configuration.'); + } finally { + await request.post(SETTINGS_URL, { data: { save_foreign_runs: true } }); } - await expect(foreign).not.toBeChecked(); - await expect(statusOf(page)).toHaveText('Unsaved changes.'); - const saved = page.waitForResponse(response => response.request().method() === 'POST' && response.url().endsWith(SETTINGS_URL)); - await saveButton(page).click(); - const response = await saved; - expect(response.request().postDataJSON()).toEqual(expect.objectContaining({ save_foreign_runs: false })); - expect((await response.json()).restart_required_fields).toEqual([]); - // Applied live: the monitor reads it for every file, so no restart is named. - await expect(statusOf(page)).toHaveText('Saved to the server configuration.'); - expect((await serverJson(request, SETTINGS_URL)).settings.save_foreign_runs).toBe(false); - // With Watch for new outputs drafted off nothing is watched: the row is disabled and says why. - const monitoring = layout.getByRole('switch', { name: 'Watch for new outputs', exact: true }); - await monitoring.uncheck(); - await expect(foreign).toBeDisabled(); - await expect(foreignRow.locator(':scope > div > p')).toHaveText(['New outputs of workflows without a PromptManager node add their prompt to the library when it is certain.', - 'Applies while Watch for new outputs is on.']); - await monitoring.check(); - await expect(foreign).toBeEnabled(); - await expect(foreignRow.locator(':scope > div > p')).toHaveCount(1); - await expect(statusOf(page)).toHaveText('No unsaved changes.'); }); test('worker range max is cpu_count', async ({ page, request }) => { diff --git a/tests/js/workspace-system-sources.test.js b/tests/js/workspace-system-sources.test.js index fe6604aa..8aa973ed 100644 --- a/tests/js/workspace-system-sources.test.js +++ b/tests/js/workspace-system-sources.test.js @@ -170,4 +170,7 @@ test('D147: "Save prompts from other workflows" drafts save_foreign_runs, disabl // Not read yet (or the host read failed): disabled without a value, the host rows' own note applies. assert.deepEqual(foreignRunsState({ value: undefined, monitoring: undefined }), { disabled: true, checked: null, note: null }); assert.deepEqual(foreignRunsState({ value: undefined, monitoring: false }), { disabled: true, checked: null, note: null }); + // Saved off while the running monitor still watches until the restart (D128 `stopping`): it can still be set. + assert.deepEqual(foreignRunsState({ value: true, monitoring: false, stopping: true }), + { disabled: false, checked: true, note: FOREIGN_RUNS.off }); }); diff --git a/web/js/workspace/system-sources.mjs b/web/js/workspace/system-sources.mjs index c7bb0a10..29f9f567 100644 --- a/web/js/workspace/system-sources.mjs +++ b/web/js/workspace/system-sources.mjs @@ -15,7 +15,8 @@ * - "Watch for new outputs" (`monitoring_enabled`) and "Worker threads" (`worker_threads`, a range up to the server's * `cpu_count`, D113) are host drafts too. "Enrich existing metadata" reviews a scan of every source. * - D147 (#247): "Save prompts from other workflows" (`save_foreign_runs`), a host draft below "Watch for new outputs", - * applied by the server at once (the monitor reads it for every file); disabled, saying why, while monitoring is off. + * applied by the server at once (the monitor reads it for every file); disabled, saying why, while monitoring is off, + * unless the running monitor still watches until a restart (D128 `stopping`): then it can still be set. * - "Thumbnail generation → Review options" opens "Thumbnail maintenance" (thumbnail-review.mjs, S5 Task 14), which * starts the thumbnail Activity job. * @@ -72,12 +73,13 @@ export const FOREIGN_RUNS = Object.freeze({ id: 'foreign-runs', key: 'save_forei /** * D147: the row for the drafted `save_foreign_runs` (`value`) and `monitoring_enabled` (`monitoring`): disabled until - * read, and while monitoring is off, when `note` says why (the value is kept and saved as it is). + * read, and while monitoring is off, when `note` says why (the value is kept and saved as it is). `stopping`: a source + * is still watched until the restart (D128), so the value can still be set; the note stays. */ -export function foreignRunsState({ value, monitoring }) { +export function foreignRunsState({ value, monitoring, stopping = false }) { if (value === undefined) return { disabled: true, checked: null, note: null }; const off = monitoring === false; - return { disabled: off, checked: value === true, note: off ? FOREIGN_RUNS.off : null }; + return { disabled: off && !stopping, checked: value === true, note: off ? FOREIGN_RUNS.off : null }; } // Labels of the sources added in this page session (validate-source answers them), for the rows not saved yet. @@ -385,15 +387,20 @@ export function mountSources(container, { api, activity, scope, drafts, notify = // D147: disabled while monitoring is drafted off, with the reason under the description. function paintForeign(row, value, unavailable) { - const state = foreignRunsState({ value, monitoring: drafts.get('host', 'monitoring_enabled') }); + const stopping = sources?.effective?.some(source => source.watch === 'stopping') === true; + const state = foreignRunsState({ value, monitoring: drafts.get('host', 'monitoring_enabled'), stopping }); row.input.disabled = state.disabled; if (state.checked !== null) row.input.checked = state.checked; const note = unavailable && value === undefined ? HOST_UNAVAILABLE : state.note; if (note && row.note?.textContent !== note) { row.note?.remove(); row.note = el('p', note, 'small'); + row.note.id = `${FOREIGN_RUNS.id}-note`; row.text.append(row.note); } else if (!note && row.note) { row.note.remove(); row.note = null; } + // The note is the switch's description, so a screen reader says why it is disabled. + if (row.note) row.input.setAttribute('aria-describedby', row.note.id); + else row.input.removeAttribute('aria-describedby'); } // The sources changed since this page read them (a refused per-source scan): read them and the saved values again. @@ -414,6 +421,7 @@ export function mountSources(container, { api, activity, scope, drafts, notify = if (answer === STALE || scope.signal.aborted) return; sources = answer?.sources ?? null; paintList(); + paintControls(); } // A save (or a reread after a conflict) changed the saved sources or monitoring: read their state again. The first