nodes/ComfyAssets-ComfyUI_PromptManager
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(system): the foreign row can be set while the monitor still watches; its note describes it (#247, D147)

Browse Source 
Review fixes: while monitoring is saved off but a source still reads
watching until restart (D128), the row stays enabled with its note; the
note is the switch's aria-describedby. The browser spec restores
save_foreign_runs on the shared server. TESTING.md moves D147 into order
and records the monitor's review rules.
This commit is contained in:
Vito Sansevero
2026-10-28 07:31:01 -07:00
parent 0d0ffc4ad8
commit be49944a63
4 changed files with 115 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
TESTING.md
+38 -22
View File
@@ -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: "<path> · <why>",
@@ -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 <file>; 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 <file>;
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 <file>; 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 <file>; left unlinked". `awaiting_foreign()` lists them;
never evicts a waiting keyless file. An expired entry is logged "No recorded run
for <file>; left unlinked", an evicted one (waiting or due) "Too many files wait for
their run; <file> 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
tests/browser/workspace-system-sources.spec.mjs
+61 -33
View File
@@ -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 }) => {
tests/js/workspace-system-sources.test.js
+3
View File
@@ -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 });
});
web/js/workspace/system-sources.mjs
+13 -5
View File
@@ -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