diff --git a/TESTING.md b/TESTING.md index 528205ee..05e0fd98 100644 --- a/TESTING.md +++ b/TESTING.md @@ -4235,16 +4235,24 @@ the per-section hand-off panel, was retired by S12 (#329): every section is nati makes the page see a restore run and finish deterministically. A refused file toasts "This file can't be restored: .". A review value may wrap anywhere (`#modal-body > dl.meta-grid dd`, CSS under D105), so a long file name keeps the concept's two equal columns at phone width. -- **D173** (#535, amends D106): "always asks" now means asks when there is a choice. One - eligible window is selected, and a selected window with one eligible node selects that - node; Preview and Apply stay explicit. A node whose requested field is linked is not - offered (`linkedRoles` on each destination), and the status line says which case applies: - linked, wrong roles, no node, or no window. Selected or not, it counts the other - nodes that cannot receive, and the review names the node and window. +- **D173** (#535, amends D106; maintainer 2026-10-30): "always asks" now means asks when + there is a choice. One eligible window is selected, and a selected window with exactly + one eligible node selects that node; two or more nodes are never selected. A node whose + requested field is linked is not offered (`linkedRoles` on each destination), and the + status line says which case applies: linked, wrong roles, no node, or no window. It + counts the other nodes that cannot receive, and the review names the node and window. - **D174** (#535): the window option shows the tab's current title, read at each discovery (a getter on the host options, which the Toolkit reads per discover), without the old - "PromptManager —" prefix. Same-named windows and nodes get " (window 2)", numbered by host id so the number - is stable while the tabs stay loaded, and " #". Discovery waits 300 ms, as System does. + "PromptManager —" prefix. Same-named windows get " (window 2)", numbered by host id so the + number is stable while the tabs stay loaded. Every node option reads "# ", so + identical nodes can be told apart. Discovery waits 300 ms, as System does. +- **D175** (#535, maintainer 2026-10-30): the concept README previews before applying; + production makes Preview optional. Apply is enabled once a window and node are selected + and requests the ticket itself, in one click. It stays closed after "Sent to ComfyUI.", + so a double click sends once, and after an unknown outcome until a new preview or + choice. The host's checks still refuse a linked field or a changed workflow, and the + sender also refuses if the destination's `revision` (node object plus graph version) + moved since discovery; it is undefined on an older tab, which skips the comparison. A host error reads as a refusal, not "may have applied". - **D106** (Integrations, S10, Q8): the send bridge always asks for the ComfyUI window and node, and the nodes always follow ComfyUI's own renderer, so "Default send destination" offers only "Ask each time" and "Host compatibility" only "Follow host diff --git a/docs/prompt-send-bridge.md b/docs/prompt-send-bridge.md index 5c5e29d6..c813f142 100644 --- a/docs/prompt-send-bridge.md +++ b/docs/prompt-send-bridge.md @@ -18,9 +18,13 @@ protocol and peer checks reject unrelated messages. Same-origin scripts are trusted; this transport is not authentication against other scripts on that origin. Unsupported messaging is reported without breaking existing ComfyUI nodes. +Apply works in one click: without a preview, the sender requests the ticket itself +and applies it. Preview remains optional, and shows the exact change first. + An apply timeout or abort after posting means the outcome is unknown. It cannot prove the host cancelled the write. The sender must inspect a fresh destination -preview before deciding to send again; automatic retry must not overwrite edits. +preview or choose the destination again before sending again; automatic retry must +not overwrite edits. An error the host returned is a refusal: nothing was written. ## PromptManager adapter @@ -31,7 +35,9 @@ field. Unknown/ambiguous metadata must not become an empty string. Previews show original and requested text plus positive prepend/append composition. Connected composition is marked unknown rather than guessing upstream values. -Connected destination fields reject writes. Applying rechecks graph, node, widgets, +Connected destination fields reject writes. The sender compares the destination's revision +(node object and graph version) from discovery with the one at the preview, and refuses if the workflow changed. +Applying rechecks graph, node, widgets, values and connections. Ordinary serialization cache synchronization is accepted only when it preserves live values. Both requested roles are checked before writing. diff --git a/tests/browser/full-inspector.spec.mjs b/tests/browser/full-inspector.spec.mjs index 424efb0b..ef2d6583 100644 --- a/tests/browser/full-inspector.spec.mjs +++ b/tests/browser/full-inspector.spec.mjs @@ -415,7 +415,7 @@ test('full inspector preserves long role text and exact sends',async({page,reque await fullFixture(page,request);const panel=inspector(page);const positive=panel.locator('.prompt-section.positive .prompt-text');await expect(positive).toContainText('猫 😀');const text=await positive.textContent();expect(text.length).toBeGreaterThan(1000); await panel.locator('.detail-footer button.send').click();const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true}); await expect(sender.getByRole('combobox',{name:'ComfyUI window',exact:true}).locator('option')).toContainText(['Exact host']); - await sender.getByRole('combobox',{name:'ComfyUI window',exact:true}).selectOption({label:'Exact host'});await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'Exact pair'}); + await sender.getByRole('combobox',{name:'ComfyUI window',exact:true}).selectOption({label:'Exact host'});await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#pair Exact pair'}); await sender.getByRole('button',{name:'Preview changes',exact:true}).click();await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click();await expect(sender).toContainText('Sent to ComfyUI.'); expect(await host.evaluate(()=>window.sent)).toEqual([{positive:text,negative:'blurred edges, watermark'}]); } finally {await host.evaluate(()=>window.bridge.dispose());await host.close();} diff --git a/tests/browser/history-canonical-comparison.mjs b/tests/browser/history-canonical-comparison.mjs index 9e1ae702..72dbc587 100644 --- a/tests/browser/history-canonical-comparison.mjs +++ b/tests/browser/history-canonical-comparison.mjs @@ -107,7 +107,7 @@ export async function exerciseCurrentPairSend(page, roles) { const send = viewer.getByRole('button', { name: /^Send prompt pair to ComfyUI: / });await send.click(); const sender = page.getByRole('dialog', { name: 'Send to ComfyUI', exact: true });await expect(sender).toBeVisible(); await sender.getByRole('combobox', { name: 'ComfyUI window', exact: true }).selectOption({ label: 'Canonical ComfyUI' }); - await sender.getByRole('combobox', { name: 'Destination node', exact: true }).selectOption({ label: 'Canonical destination' }); + await sender.getByRole('combobox', { name: 'Destination node', exact: true }).selectOption({ label: '#canonical-pair Canonical destination' }); await sender.getByRole('button', { name: 'Preview changes', exact: true }).click(); await expect(sender).toContainText(roles.positive);await expect(sender).toContainText(roles.negative); await sender.getByRole('button', { name: 'Apply to ComfyUI', exact: true }).click();await expect(sender).toContainText('Sent to ComfyUI.'); diff --git a/tests/browser/history-concept.spec.mjs b/tests/browser/history-concept.spec.mjs index ffd96385..01bed46c 100644 --- a/tests/browser/history-concept.spec.mjs +++ b/tests/browser/history-concept.spec.mjs @@ -194,7 +194,7 @@ function productionSent(host) { return async page=>{ const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true}); await sender.getByRole('combobox',{name:'ComfyUI window',exact:true}).selectOption({label:'Concept ComfyUI'}); - await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'Concept destination'}); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#pair-1 Concept destination'}); await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await expect.poll(()=>host.evaluate(()=>window.previewed.length)).toBe(1); const roles=await host.evaluate(()=>window.previewed.pop()); diff --git a/tests/browser/inspector-concept.spec.mjs b/tests/browser/inspector-concept.spec.mjs index 2b1467ef..a889e945 100644 --- a/tests/browser/inspector-concept.spec.mjs +++ b/tests/browser/inspector-concept.spec.mjs @@ -259,7 +259,7 @@ async function productionSendOf(page,context) { const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true});await expect(sender).toBeVisible(); const windows=sender.getByRole('combobox',{name:'ComfyUI window',exact:true}); await expect(windows.locator('option')).toContainText(['Test ComfyUI']);await windows.selectOption({label:'Test ComfyUI'}); - await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'My prompt pair'}); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#pair-1 My prompt pair'}); await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click(); await expect(sender).toContainText('Sent to ComfyUI.'); diff --git a/tests/browser/library-concept.spec.mjs b/tests/browser/library-concept.spec.mjs index d7387aaf..b6887c86 100644 --- a/tests/browser/library-concept.spec.mjs +++ b/tests/browser/library-concept.spec.mjs @@ -233,7 +233,7 @@ function productionSent(host) { return async page=>{ const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true}); await sender.getByRole('combobox',{name:'ComfyUI window',exact:true}).selectOption({label:'Concept ComfyUI'}); - await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'Concept destination'}); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#pair-1 Concept destination'}); await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await expect.poll(()=>host.evaluate(()=>window.previewed.length)).toBe(1); const roles=await host.evaluate(()=>window.previewed.pop()); diff --git a/tests/browser/prompt-inspector.spec.mjs b/tests/browser/prompt-inspector.spec.mjs index 40bb405c..56d9c8ec 100644 --- a/tests/browser/prompt-inspector.spec.mjs +++ b/tests/browser/prompt-inspector.spec.mjs @@ -33,7 +33,7 @@ async function chooseDestination(page) { const host=sender.getByRole('combobox',{name:'ComfyUI window',exact:true}); await expect(host.locator('option')).toContainText(['Test ComfyUI']); await host.selectOption({label:'Test ComfyUI'}); - await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'My prompt pair'}); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#pair-1 My prompt pair'}); return sender; } @@ -165,7 +165,8 @@ test('host apply rejection retains the current text and requires a new preview', await page.goto(workspace);await select(page,'0:shared/same.png');await details(page).getByRole('button',{name:/^Send positive prompt to ComfyUI: /}).click(); const sender=await chooseDestination(page);await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click();await expect(sender).toContainText(/Workflow changed/); - await expect(sender.getByRole('button',{name:'Apply to ComfyUI',exact:true})).toBeDisabled(); + // D175: a refusal wrote nothing, so Apply may try again. + await expect(sender.getByRole('button',{name:'Apply to ComfyUI',exact:true})).toBeEnabled(); expect(await host.evaluate(()=>window.hostValues.positive)).toBe('Existing positive'); } finally {await host.evaluate(()=>window.fixtureHost.dispose());await host.close();} }); diff --git a/tests/browser/system-integrations-concept.spec.mjs b/tests/browser/system-integrations-concept.spec.mjs index 6e3ffa13..c70788ef 100644 --- a/tests/browser/system-integrations-concept.spec.mjs +++ b/tests/browser/system-integrations-concept.spec.mjs @@ -88,7 +88,7 @@ function measureIntegrations(mainSelector) { }; } -const SEND_REASON = 'Sending asks when there is more than one ComfyUI window or node and always previews, so “Ask each time” is the only option.'; +const SEND_REASON = 'Sending asks when there is more than one ComfyUI window or node, so “Ask each time” is the only option.'; const RENDERER_REASON = 'Prompt Manager nodes always follow ComfyUI’s own renderer setting.'; const RENDERER_NONE = 'No ComfyUI window is open in this browser, so its renderer is not known.'; const RETURN_REASON = 'Not available: sending applies the text in ComfyUI and keeps this page open.'; diff --git a/tests/browser/workspace-history.spec.mjs b/tests/browser/workspace-history.spec.mjs index d24e391b..8591c11c 100644 --- a/tests/browser/workspace-history.spec.mjs +++ b/tests/browser/workspace-history.spec.mjs @@ -468,7 +468,7 @@ test('frozen Effective pair uses the shared acknowledged sender and Escape close const positive=await exact(dialog(page).getByRole('region',{name:'Positive prompt',exact:true}).locator('p.prompt-text')); const send=dialog(page).getByRole('button',{name:'Send prompt pair',exact:true});await send.click(); const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true});await sender.getByRole('combobox',{name:'ComfyUI window',exact:true}).selectOption({label:'History ComfyUI'}); - await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'History destination'});await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#history-pair History destination'});await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await expect(sender.getByRole('button',{name:'Apply to ComfyUI',exact:true})).toBeEnabled();await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click();await expect(sender).toContainText('Sent to ComfyUI.'); expect(await host.evaluate(()=>window.historyApplied)).toEqual([{positive,negative:' \n\t '}]); await page.keyboard.press('Escape');await expect(sender).toHaveCount(0);await expect(dialog(page)).toBeVisible();await expect(send).toBeFocused(); diff --git a/tests/browser/workspace-inspect.spec.mjs b/tests/browser/workspace-inspect.spec.mjs index 81795e44..494899ed 100644 --- a/tests/browser/workspace-inspect.spec.mjs +++ b/tests/browser/workspace-inspect.spec.mjs @@ -975,7 +975,7 @@ async function sendPair(page) { await rail(page).getByRole('button', { name: /^Send prompt pair to ComfyUI: / }).click(); const sender = page.getByRole('dialog', { name: 'Send to ComfyUI', exact: true }); await sender.getByRole('combobox', { name: 'ComfyUI window', exact: true }).selectOption({ label: 'Inspect ComfyUI' }); - await sender.getByRole('combobox', { name: 'Destination node', exact: true }).selectOption({ label: 'Inspect destination' }); + await sender.getByRole('combobox', { name: 'Destination node', exact: true }).selectOption({ label: '#pair-1 Inspect destination' }); await sender.getByRole('button', { name: 'Preview changes', exact: true }).click(); await sender.getByRole('button', { name: 'Apply to ComfyUI', exact: true }).click(); await expect(sender).toContainText('Sent to ComfyUI.'); diff --git a/tests/browser/workspace-library.spec.mjs b/tests/browser/workspace-library.spec.mjs index a2ba6187..146a8cca 100644 --- a/tests/browser/workspace-library.spec.mjs +++ b/tests/browser/workspace-library.spec.mjs @@ -364,7 +364,7 @@ test('history shares real image metadata, vendored Viewer and the acknowledged p const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true}); const hosts=sender.getByRole('combobox',{name:'ComfyUI window',exact:true}); await expect(hosts.locator('option')).toHaveCount(2);await hosts.selectOption({label:'Library ComfyUI'}); - await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'Library destination'}); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#pair-1 Library destination'}); await sender.getByRole('button',{name:'Preview changes',exact:true}).click();await expect(sender).toContainText(chosenNegative||'(empty)'); await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click();await expect(sender).toContainText('Sent to ComfyUI.'); expect(await host.evaluate(()=>window.libraryApplied)).toEqual([{positive:'Cat portrait',negative:chosenNegative}]); @@ -716,7 +716,7 @@ test('long prompt is clamped to two lines while copy sends the full text',async( await card.locator('.card-footer .row-actions > button.send').click(); const sender=page.getByRole('dialog',{name:'Send to ComfyUI',exact:true}); await sender.getByRole('combobox',{name:'ComfyUI window',exact:true}).selectOption({label:'Card ComfyUI'}); - await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'Card destination'}); + await sender.getByRole('combobox',{name:'Destination node',exact:true}).selectOption({label:'#positive-1 Card destination'}); await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); await sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}).click();await expect(sender).toContainText('Sent to ComfyUI.'); expect(await host.evaluate(()=>window.cardApplied)).toEqual([{positive:longPrompt}]); diff --git a/tests/browser/workspace-system-integrations.spec.mjs b/tests/browser/workspace-system-integrations.spec.mjs index 7cb18626..c9331e1a 100644 --- a/tests/browser/workspace-system-integrations.spec.mjs +++ b/tests/browser/workspace-system-integrations.spec.mjs @@ -99,7 +99,7 @@ test('send destination offers Ask each time; unsupported options disabled with t .toEqual([['Ask each time', false], ['Selected Prompt Manager node', true], ['Create a Prompt Manager (+/-) node', true]]); await expect(send.locator('select')).toHaveValue('Ask each time'); await expect(send.locator('p.small')).toHaveText( - 'Sending asks when there is more than one ComfyUI window or node and always previews, so “Ask each time” is the only option.'); + 'Sending asks when there is more than one ComfyUI window or node, so “Ask each time” is the only option.'); const renderer = settingOf(page, 'Host compatibility'); expect(await renderer.locator('option').evaluateAll(options => options.map(option => [option.textContent, option.disabled]))) .toEqual([['Follow host renderer', false], ['Classic', true], ['Nodes 2.0', true]]); diff --git a/tests/comfyui/prompt-sender.spec.mjs b/tests/comfyui/prompt-sender.spec.mjs index 58763e73..733b7ba5 100644 --- a/tests/comfyui/prompt-sender.spec.mjs +++ b/tests/comfyui/prompt-sender.spec.mjs @@ -74,7 +74,8 @@ test('native image inspector previews and sends a pair into real visible widgets expect(tabTitle).not.toContain('PromptManager'); await expect(hosts).not.toHaveValue(''); await expect(sender.getByRole('combobox',{name:'Destination node',exact:true})).not.toHaveValue(''); - await expect(sender.getByRole('status')).toContainText(`in ${tabTitle}. Preview changes before applying.`); + await expect(sender.getByRole('status')).toContainText(`in ${tabTitle}. Apply to send it, or preview the changes first.`); + await expect(sender.getByRole('combobox',{name:'Destination node',exact:true}).locator('option').nth(1)).toHaveText(new RegExp(`^#${id} `)); await sender.getByRole('button',{name:'Preview changes',exact:true}).click(); for(const text of ['Before positive','Before negative',positive,negative,`LoRA trigger ${positive} fine detail`])await expect(sender).toContainText(text); await expect(nodeRoot.getByRole('textbox',{name:'Positive prompt',exact:true})).toHaveValue('Before positive'); @@ -91,6 +92,50 @@ test('native image inspector previews and sends a pair into real visible widgets for(const name of ['inputs','serialized'])expect(state[name]).toMatchObject({text:positive,negative_text:negative,prepend_text:'LoRA trigger',append_text:'fine detail'}); expect(state.effects).toEqual({queue:0,load:0});expect(errors).toEqual([]); await workspace.screenshot({path:testInfo.outputPath('full-inspector-host-send.png')}); + // #535 D175: Apply without Preview, one click, and a double click sends once. + const apply=sender.getByRole('button',{name:'Apply to ComfyUI',exact:true}); + const find0=sender.getByRole('button',{name:'Find ComfyUI windows',exact:true}); + const positiveBox=nodeRoot.getByRole('textbox',{name:'Positive prompt',exact:true}); + await positiveBox.fill('Typed before one click'); + await find0.click(); + await expect(apply).toBeEnabled(); + await apply.dblclick(); + await expect(sender).toContainText('Sent to ComfyUI.'); + await expect(positiveBox).toHaveValue(positive); + await expect(apply).toBeDisabled(); + // Edited in ComfyUI after the send: a stray second send would overwrite this. + await positiveBox.fill('Typed after the send'); + await expect(positiveBox).toHaveValue('Typed after the send'); + // A changed workflow since discovery refuses through one-click Apply, and writes nothing. + await find0.click(); + await expect(apply).toBeEnabled(); + await page.evaluate(async()=>{const {app}=await import('/scripts/app.js');await app.loadGraphData(app.graph.serialize());}); + await apply.click(); + await expect(sender.getByRole('alert')).toContainText(/workflow changed/i); + await expect(positiveBox).toHaveValue('Typed after the send'); + // A field linked after discovery refuses the same way. + await find0.click(); + await expect(apply).toBeEnabled(); + await page.evaluate(async id=>{ + const {app}=await import('/scripts/app.js');const {convertToInput}=await import('/extensions/core/widgetInputs.js'); + const node=app.graph.getNodeById(id);const widget=node.widgets.find(widget=>widget.name==='negative_text'); + if(!node.inputs.some(input=>input.name==='negative_text'))convertToInput(node,widget); + const source=LiteGraph.createNode('PrimitiveString');app.graph.add(source); + source.connect(0,node,node.inputs.findIndex(input=>input.name==='negative_text')); + },id); + await apply.click(); + await expect(sender.getByRole('alert')).toContainText(/connected/i); + await expect(positiveBox).toHaveValue('Typed after the send'); + await page.evaluate(async id=>{const {app}=await import('/scripts/app.js');const node=app.graph.getNodeById(id);node.disconnectInput(node.inputs.findIndex(input=>input.name==='negative_text'));},id); + // Two identical nodes are never selected for you, and read "#<id> <title>". + const twinId=await page.evaluate(async()=>{const {app}=await import('/scripts/app.js');const node=LiteGraph.createNode('PromptManagerPair');app.graph.add(node);return node.id;}); + await find0.click(); + const targets=sender.getByRole('combobox',{name:'Destination node',exact:true}); + await expect(targets.locator('option')).toHaveCount(3); + await expect(targets).toHaveValue(''); + await expect(apply).toBeDisabled(); + await expect(targets.locator('option').nth(2)).toHaveText(new RegExp(`^#${twinId} `)); + await page.evaluate(async twinId=>{const {app}=await import('/scripts/app.js');app.graph.remove(app.graph.getNodeById(twinId));},twinId); // #535: the status line says why a window cannot receive. const find=sender.getByRole('button',{name:'Find ComfyUI windows',exact:true}); const preview=sender.getByRole('button',{name:'Preview changes',exact:true}); diff --git a/tests/js/prompt-bridge-adapter.test.js b/tests/js/prompt-bridge-adapter.test.js index 82238b48..f9b4d560 100644 --- a/tests/js/prompt-bridge-adapter.test.js +++ b/tests/js/prompt-bridge-adapter.test.js @@ -159,3 +159,14 @@ test('listTargets reports which roles are linked to an upstream node',async()=>{ const [pair,plain]=createPromptAdapter(app,{runtime:{}}).listTargets(); assert.deepEqual(pair.linkedRoles,['negative']);assert.deepEqual(plain.linkedRoles,[]); }); + +test('the revision moves when the graph is edited and when a load replaces the node, even at the same version',async()=>{ + const {createPromptAdapter}=await import('../../web/js/prompt-bridge-adapter.js'); + const node=()=>({id:1,type:'PromptManager',widgets:[{name:'text',value:''}],inputs:[]}); + const app={graph:{_version:9,_nodes:[node()]}}; + const adapter=createPromptAdapter(app,{runtime:{}}); + const first=adapter.listTargets()[0].revision; + assert.equal(adapter.preview('1',{positive:'x'}).preview.revision,first); + app.graph._version=10;const edited=adapter.listTargets()[0].revision;assert.notEqual(edited,first); + app.graph={_version:10,_nodes:[node()]};assert.notEqual(adapter.listTargets()[0].revision,edited); +}); diff --git a/tests/js/prompt-sender.test.js b/tests/js/prompt-sender.test.js index 19b7d1ce..6ab76553 100644 --- a/tests/js/prompt-sender.test.js +++ b/tests/js/prompt-sender.test.js @@ -14,7 +14,6 @@ test('sender requires explicit compatible host and target, preview, then acknowl await s.session.discover(); assert.equal(s.session.state.hostId,'host');assert.equal(s.session.state.targetId,''); await s.session.preview(); assert.equal(s.calls.length,0); s.session.selectTarget('one'); - await s.session.apply();assert.equal(s.calls.length,0); await s.session.preview();assert.equal(s.session.state.phase,'ready');assert.equal(s.session.state.preview.composition.after,'prefix new'); await s.session.apply();assert.equal(s.session.state.phase,'success');assert.equal(s.calls.filter(call=>call[0]==='apply').length,1); await s.session.apply();assert.equal(s.calls.filter(call=>call[0]==='apply').length,1); s.session.dispose(); @@ -66,7 +65,7 @@ async function discovered(createSendSession,found,roles={positive:'new'}) { test('one window with one eligible node is selected so Preview is available at once',async()=>{ const {createSendSession}=await source;const s=await discovered(createSendSession,[window1([pm('1')])]); assert.equal(s.session.state.hostId,'a');assert.equal(s.session.state.targetId,'1'); - assert.match(s.session.state.message,/Node 1 in Flow - ComfyUI.*Preview changes/); + assert.match(s.session.state.message,/#1 Node 1 in Flow - ComfyUI.*Apply/); await s.session.preview();assert.equal(s.session.state.phase,'ready');s.session.dispose(); }); test('several windows are left to choose, and choosing one with a single node selects that node',async()=>{ @@ -101,17 +100,69 @@ test('an auto-selected destination says how many other nodes cannot receive, and assert.equal(s.session.state.targetId,'2');assert.match(s.session.state.message,/1 other Prompt Manager node cannot receive/); await s.session.preview();s.session.dispose(); const named=setup(createSendSession,{discover:async()=>[window1([pm('one')])],preview:async(_,id)=>({ticket:'t',target:{id,label:'Live title'},fields:[{role:'positive',before:'',after:'new'}]})}); - await named.session.discover();await named.session.preview();assert.match(named.session.state.message,/Sending to Live title in Flow - ComfyUI/);named.session.dispose(); + await named.session.discover();await named.session.preview();assert.match(named.session.state.message,/Sending to #one Live title in Flow - ComfyUI/);named.session.dispose(); }); test('window order and numbering do not depend on which tab answered first',async()=>{ const {createSendSession}=await source;const s=await discovered(createSendSession,[window1([pm('1')],'T','b'),window1([pm('2')],'T','a')]); assert.deepEqual(s.session.state.hosts.map(host=>[host.hostId,host.label]),[['a','T'],['b','T (window 2)']]);s.session.dispose(); }); +const applied=calls=>calls.filter(call=>call[0]==='apply').length; +test('Apply without Preview fetches the ticket and applies in one click',async()=>{ + const {createSendSession}=await source;const s=setup(createSendSession,{discover:async()=>[window1([pm('one')])]}); + await s.session.discover();await s.session.apply(); + assert.equal(s.session.state.phase,'success');assert.deepEqual(s.calls.map(call=>call[0]),['preview','apply']);s.session.dispose(); +}); +test('a double click applies once, during the send and after it',async()=>{ + const {createSendSession}=await source;const s=setup(createSendSession,{discover:async()=>[window1([pm('one')])]}); + await s.session.discover();const first=s.session.apply();const second=s.session.apply();await Promise.all([first,second]); + assert.equal(applied(s.calls),1);await s.session.apply();assert.equal(applied(s.calls),1);s.session.dispose(); +}); +test('Apply after a preview uses its ticket and does not preview again',async()=>{ + const {createSendSession}=await source;const s=setup(createSendSession,{discover:async()=>[window1([pm('one')])]}); + await s.session.discover();await s.session.preview();await s.session.apply(); + assert.deepEqual(s.calls.map(call=>call[0]),['preview','apply']);assert.equal(s.session.state.phase,'success');s.session.dispose(); +}); +test('Apply needs a destination',async()=>{ + const {createSendSession}=await source;const s=setup(createSendSession); + await s.session.discover();await s.session.apply();assert.equal(s.calls.length,0);s.session.dispose(); +}); +test('a refusal at the fetched preview says so plainly and writes nothing; Apply can try again',async()=>{ + const {createSendSession}=await source;let refuse=true; + const s=setup(createSendSession,{discover:async()=>[window1([pm('one')])],preview:async(_,id)=>{if(refuse)throw Error('positive is connected to an upstream input.');return{ticket:'t',target:{id},fields:[{role:'positive',before:'',after:'new'}]};}}); + await s.session.discover();await s.session.apply(); + assert.equal(s.session.state.phase,'error');assert.match(s.session.state.message,/connected/);assert.doesNotMatch(s.session.state.message,/may have applied/); + refuse=false;await s.session.apply();assert.equal(s.session.state.phase,'success');s.session.dispose(); +}); +test('a host refusal at apply is a refusal; only a lost reply is an unknown outcome',async()=>{ + const {createSendSession}=await source; + const s=setup(createSendSession,{discover:async()=>[window1([pm('one')])],apply:async()=>{throw Error('Workflow or prompt changed. Preview again.');}}); + await s.session.discover();await s.session.apply();assert.match(s.session.state.message,/Workflow or prompt changed/);assert.doesNotMatch(s.session.state.message,/may have applied/);s.session.dispose(); + const lost=setup(createSendSession,{discover:async()=>[window1([pm('one')])],apply:async()=>{throw Error('Bridge request timed out');}}); + await lost.session.discover();await lost.session.apply();assert.match(lost.session.state.message,/may have applied/); + await lost.session.apply();assert.equal(lost.calls.filter(call=>call[0]==='preview').length,1); + lost.session.selectTarget('one');await lost.session.apply();assert.equal(lost.calls.filter(call=>call[0]==='preview').length,2);lost.session.dispose(); +}); +test('a workflow changed since discovery refuses before anything is written',async()=>{ + const {createSendSession}=await source; + for(const op of ['apply','preview']){ + const s=setup(createSendSession,{discover:async()=>[window1([pm('one',['positive'],{revision:'1:4'})])],preview:async(_,id)=>({ticket:'t',revision:'1:5',target:{id},fields:[{role:'positive',before:'',after:'new'}]})}); + await s.session.discover();await s.session[op](); + assert.equal(s.session.state.phase,'error');assert.match(s.session.state.message,/workflow changed/i);assert.equal(applied(s.calls),0);s.session.dispose(); + } + const stale=setup(createSendSession,{discover:async()=>[window1([pm('one')])]});await stale.session.discover();await stale.session.apply();assert.equal(stale.session.state.phase,'success');stale.session.dispose(); +}); +test('two or more eligible nodes are never auto-selected, and each option shows id and title',async()=>{ + const {createSendSession}=await source;const twin=id=>({id,label:'Prompt Manager (+/-)',roles:['positive']}); + const s=await discovered(createSendSession,[window1([twin('72'),twin('73')])]); + assert.equal(s.session.state.targetId,'');assert.deepEqual(s.session.state.hosts[0].targets.map(target=>target.label),['#72 Prompt Manager (+/-)','#73 Prompt Manager (+/-)']); + await s.session.apply();assert.equal(s.calls.length,0);s.session.dispose(); + const one=await discovered(createSendSession,[window1([twin('72')])]);assert.equal(one.session.state.hosts[0].targets[0].label,'#72 Prompt Manager (+/-)');assert.equal(one.session.state.targetId,'72');one.session.dispose(); +}); test('same-named windows and nodes stay distinguishable',async()=>{ const {createSendSession}=await source;const twin=id=>({id,label:'Same',roles:['positive']}); const s=await discovered(createSendSession,[window1([twin('7'),twin('8')],'T','a'),window1([pm('1')],'T','b')]); assert.deepEqual(s.session.state.hosts.map(host=>host.label),['T','T (window 2)']); - assert.deepEqual(s.session.state.hosts[0].targets.map(target=>target.label),['Same #7','Same #8']);s.session.dispose(); + assert.deepEqual(s.session.state.hosts[0].targets.map(target=>target.label),['#7 Same','#8 Same']);s.session.dispose(); }); function fakeDOM() { class Node extends EventTarget { diff --git a/web/js/prompt-bridge-adapter.js b/web/js/prompt-bridge-adapter.js index c61488aa..dc4e37c9 100644 --- a/web/js/prompt-bridge-adapter.js +++ b/web/js/prompt-bridge-adapter.js @@ -58,9 +58,16 @@ function renderer(runtime) { export function createPromptAdapter(app, {runtime = globalThis} = {}) { const snapshots = new WeakMap(); + // Loading a workflow makes new node objects but can land on the same graph version, so a revision needs both. + const nodeSerials = new WeakMap(); + let lastSerial = 0; + function revision(node) { + if (!nodeSerials.has(node)) nodeSerials.set(node,++lastSerial); + return `${nodeSerials.get(node)}:${app.graph?._version}`; + } function listTargets() { const drawn = renderer(runtime); - return nodes(app.graph).filter(node => schemas.has(node.type)).map(node => ({...target(node),roles:Object.keys(schemas.get(node.type)),linkedRoles:linkedRoles(node),...drawn})); + return nodes(app.graph).filter(node => schemas.has(node.type)).map(node => ({...target(node),roles:Object.keys(schemas.get(node.type)),linkedRoles:linkedRoles(node),revision:revision(node),...drawn})); } function preview(targetId,rawRoles) { const roles = validateRoles(rawRoles); @@ -78,7 +85,7 @@ export function createPromptAdapter(app, {runtime = globalThis} = {}) { }); const snapshot = Object.freeze({}); snapshots.set(snapshot,{graph,node,type:node.type,id:String(node.id),version:graph._version,widgets:node.widgets,widgetOrder:[...node.widgets],fields,links:links(node),requested}); - return {preview:{target:target(node),fields:requested.map(({role,after,field}) => ({role,before:String(field.value ?? ''),after})),composition:composition(node,fields,roles)},snapshot}; + return {preview:{target:target(node),revision:revision(node),fields:requested.map(({role,after,field}) => ({role,before:String(field.value ?? ''),after})),composition:composition(node,fields,roles)},snapshot}; } function apply(snapshot) { const state = snapshots.get(snapshot); diff --git a/web/js/workspace/prompt-sender.mjs b/web/js/workspace/prompt-sender.mjs index b95f216e..08faf726 100644 --- a/web/js/workspace/prompt-sender.mjs +++ b/web/js/workspace/prompt-sender.mjs @@ -8,12 +8,16 @@ const roleOf = target => Array.isArray(target.roles) ? target.roles : []; const accepts = (target,roles) => typeof target.id==='string' && Object.keys(roles).every(role=>roleOf(target).includes(role)); const linkedTo = (target,roles) => Object.keys(roles).filter(role=>Array.isArray(target.linkedRoles) && target.linkedRoles.includes(role)); +const nodeLabel = target => `#${target.id} ${target.label}`; +// A lost reply leaves the outcome unknown; anything else the host said is a refusal, and nothing was written. +const LOST_REPLY = /timed out|aborted|disposed/i; + /** Why nothing in the discovered windows can receive these roles, most specific first (D173). */ function unavailableMessage(found,roles) { if (!found.length) return 'No ComfyUI window answered in this browser at this address. Open ComfyUI here, then find windows again.'; for (const host of found) for (const target of host.targets) { const linked = accepts(target,roles) ? linkedTo(target,roles) : []; - if (linked.length) return `${target.label} in ${host.label} has its ${linked.join(' and ')} prompt linked to another node, so it cannot receive text. Disconnect the link, then find windows again.`; + if (linked.length) return `${nodeLabel(target)} in ${host.label} has its ${linked.join(' and ')} prompt linked to another node, so it cannot receive text. Disconnect the link, then find windows again.`; } const where = found.length===1 ? found[0].label : `None of the ${found.length} ComfyUI windows`; const verb = found.length===1 ? 'has no' : 'has a'; @@ -23,7 +27,7 @@ function unavailableMessage(found,roles) { } return `${where} ${verb} Prompt Manager node. Add one to the workflow, then find windows again.`; } -/** Keeps same-named options apart: tabs on one workflow, or nodes that share a title. */ +/** Keeps same-named windows apart: tabs on one workflow. */ function distinctLabels(items,suffix) { const seen = new Map(); const totals = new Map(); @@ -36,17 +40,27 @@ function distinctLabels(items,suffix) { export function createSendSession({client, roles:rawRoles, onChange}) { const roles = Object.freeze(validateRoles(rawRoles)); - let state = {phase:'idle',hosts:[],hostId:'',targetId:'',preview:null,message:''}; + let state = {phase:'idle',hosts:[],hostId:'',targetId:'',preview:null,outcomeUnknown:false,message:''}; let operation, generation = 0, disposed = false; const emit = patch => { if (!disposed) { state = Object.freeze({...state,...patch}); onChange(state); } }; function invalidate() { generation++; operation?.abort(); operation = null; } - function begin(phase) { invalidate(); operation = new AbortController(); emit({phase,preview:null,message:''}); return {signal:operation.signal,version:generation}; } + function begin(phase) { invalidate(); operation = new AbortController(); emit({phase,preview:null,outcomeUnknown:false,message:''}); return {signal:operation.signal,version:generation}; } const current = request => !disposed && generation === request.version; const selected = () => state.hosts.find(host=>host.hostId===state.hostId)?.targets.find(target=>target.id===state.targetId); + async function fetchPreview(request) { + const preview=await client.preview(state.hostId,state.targetId,roles,{signal:request.signal}); + if (!current(request)) return null; + const fields=preview?.fields; + if(typeof preview?.ticket!=='string' || !preview.ticket || preview.target?.id!==state.targetId || !Array.isArray(fields) || fields.length!==Object.keys(roles).length || new Set(fields.map(field=>field?.role)).size!==fields.length || Array.from(fields).some(field=>!field || !Object.hasOwn(roles,field.role) || typeof field.before!=='string' || field.after!==roles[field.role])) throw Error('ComfyUI returned an unexpected preview. Find the destination and preview again.'); + // Undefined on a tab still running older code, which has nothing to compare. + const found=selected()?.revision; + if(found!==undefined && preview.revision!==found) throw Error('The workflow changed since this window was found. Nothing was sent. Find windows again.'); + return preview; + } function hostChoice(host) { if (host.targets.length>1) return {hostId:host.hostId,targetId:'',message:`Choose a destination node in ${host.label}.`}; const [target] = host.targets; - return {hostId:host.hostId,targetId:target.id,message:`Sending to ${target.label} in ${host.label}. Preview changes before applying.`}; + return {hostId:host.hostId,targetId:target.id,message:`Sending to ${target.label} in ${host.label}. Apply to send it, or preview the changes first.`}; } return { get state() { return state; }, @@ -56,7 +70,7 @@ export function createSendSession({client, roles:rawRoles, onChange}) { try { const found = (await client.discover({signal:request.signal,waitMs:DISCOVERY_WAIT_MS})).map(host=>({...host,targets:host.targets.filter(target=>target && typeof target==='object')})); if (!current(request)) return; - const hosts = distinctLabels(found.toSorted((a,b)=>a.hostId<b.hostId?-1:1).map(host=>({...host,targets:distinctLabels(host.targets.filter(target=>accepts(target,roles) && !linkedTo(target,roles).length),(target)=>`${target.label} #${target.id}`)})).filter(host=>host.targets.length),(host,count)=>count>1 ? `${host.label} (window ${count})` : host.label); + const hosts = distinctLabels(found.toSorted((a,b)=>a.hostId<b.hostId?-1:1).map(host=>({...host,targets:host.targets.filter(target=>accepts(target,roles) && !linkedTo(target,roles).length).map(target=>({...target,label:nodeLabel(target)}))})).filter(host=>host.targets.length),(host,count)=>count>1 ? `${host.label} (window ${count})` : host.label); // Nodes left out are counted so an auto-selected destination never hides the one the user meant. const skipped = found.reduce((total,host)=>total+host.targets.length,0)-hosts.reduce((total,host)=>total+host.targets.length,0); const note = skipped ? ` ${skipped} other Prompt Manager ${skipped===1?'node':'nodes'} cannot receive this prompt.` : ''; @@ -65,31 +79,39 @@ export function createSendSession({client, roles:rawRoles, onChange}) { else { const choice=hostChoice(hosts[0]); emit({phase:'select',hosts,...choice,message:choice.message+note}); } } catch (error) { if (current(request)) emit({phase:'error',message:error.message}); } }, - selectHost(hostId) { if (disposed) return; invalidate(); const host=state.hosts.find(host=>host.hostId===hostId); emit({phase:'select',preview:null,...(host ? hostChoice(host) : {hostId:'',targetId:'',message:'Choose a ComfyUI window.'})}); }, - selectTarget(targetId) { if (disposed) return; invalidate(); const host=state.hosts.find(host=>host.hostId===state.hostId); const chosen=host?.targets.some(target=>target.id===targetId); emit({phase:'select',targetId:chosen?targetId:'',preview:null,message:chosen?'Preview changes before applying.':'Choose a destination node, then preview changes.'}); }, + selectHost(hostId) { if (disposed) return; invalidate(); const host=state.hosts.find(host=>host.hostId===hostId); emit({phase:'select',preview:null,outcomeUnknown:false,...(host ? hostChoice(host) : {hostId:'',targetId:'',message:'Choose a ComfyUI window.'})}); }, + selectTarget(targetId) { if (disposed) return; invalidate(); const host=state.hosts.find(host=>host.hostId===state.hostId); const chosen=host?.targets.some(target=>target.id===targetId); emit({phase:'select',outcomeUnknown:false,targetId:chosen?targetId:'',preview:null,message:chosen?'Apply to send it, or preview the changes first.':'Choose a destination node, then preview changes.'}); }, async preview() { if (disposed || !selected() || ['previewing','applying'].includes(state.phase)) return; const request=begin('previewing'); try { - const preview=await client.preview(state.hostId,state.targetId,roles,{signal:request.signal}); - if (!current(request)) return; - const fields=preview?.fields; - if(typeof preview?.ticket!=='string' || !preview.ticket || preview.target?.id!==state.targetId || !Array.isArray(fields) || fields.length!==Object.keys(roles).length || new Set(fields.map(field=>field?.role)).size!==fields.length || Array.from(fields).some(field=>!field || !Object.hasOwn(roles,field.role) || typeof field.before!=='string' || field.after!==roles[field.role])) throw Error('ComfyUI returned an unexpected preview. Find the destination and preview again.'); + const preview=await fetchPreview(request); + if (!preview) return; const windowLabel=state.hosts.find(host=>host.hostId===state.hostId)?.label; - emit({phase:'ready',preview,message:`Sending to ${preview.target.label ?? state.targetId} in ${windowLabel}. Review the complete changes, then apply.`}); + emit({phase:'ready',preview,message:`Sending to ${nodeLabel({id:preview.target.id,label:preview.target.label ?? ''})} in ${windowLabel}. Review the complete changes, then apply.`}); } catch(error) { if(current(request)) emit({phase:'error',preview:null,message:error.message}); } }, async apply() { - if(disposed || state.phase!=='ready' || !state.preview) return; - const {ticket}=state.preview; const targetId=state.targetId; const hostId=state.hostId; + // Success stays closed: the second click of a double click must not send again. + if(disposed || !selected() || !['select','ready','error'].includes(state.phase) || state.outcomeUnknown) return; + const held=state.phase==='ready' ? state.preview : null; + const targetId=state.targetId; const hostId=state.hostId; const request=begin('applying'); emit({message:'Waiting for ComfyUI confirmation. Closing this dialog does not undo a sent update.'}); + let posted=false; try { - const acknowledgement=await client.apply(hostId,ticket,{signal:request.signal}); + const preview=held ?? await fetchPreview(request); + if(!preview || !current(request)) return; + posted=true; + const acknowledgement=await client.apply(hostId,preview.ticket,{signal:request.signal}); if(!current(request)) return; if(acknowledgement?.targetId!==targetId || !acknowledgement.values || Object.keys(acknowledgement.values).length!==Object.keys(roles).length || Object.keys(roles).some(role=>!Object.hasOwn(acknowledgement.values,role) || acknowledgement.values[role]!==roles[role])) throw Error('ComfyUI returned an unexpected acknowledgement.'); emit({phase:'success',message:'Sent to ComfyUI.'}); - } catch(error) { if(current(request)) emit({phase:'error',preview:null,message:`No confirmation received. The update may have applied. Check ComfyUI, then preview again. ${error.message}`}); } + } catch(error) { + if(!current(request)) return; + const unknown=posted && (LOST_REPLY.test(error.message) || /unexpected acknowledgement/.test(error.message)); + emit({phase:'error',preview:null,outcomeUnknown:unknown,message:unknown ? `No confirmation received. The update may have applied. Check ComfyUI, then preview again. ${error.message}` : error.message}); + } }, dispose() { if(disposed) return; disposed=true; invalidate(); client.dispose(); }, }; @@ -126,7 +148,7 @@ export function createPromptSender({scope,baseUrl=window.ApiClient.resolveUrl('/ const header=element('header',undefined,'prompt-sender-header'); const title=element('h2','Send to ComfyUI');const dismiss=createButton('Close'); header.append(title,dismiss); - const description=element('p','Choose where to send this prompt. Nothing changes until you approve the preview.'); + const description=element('p','Choose where to send this prompt. Apply writes it to that node. Preview changes shows the result first, if you want to check it.'); const status=element('p','', 'prompt-sender-status');status.setAttribute('role','status');status.setAttribute('aria-live','polite'); const host=createSelect([['','Choose a ComfyUI window']], ''); const target=createSelect([['','Choose a destination node']], ''); @@ -167,7 +189,7 @@ export function createPromptSender({scope,baseUrl=window.ApiClient.resolveUrl('/ const selected=state.hosts.find(item=>item.hostId===state.hostId); options(target,'Choose a destination node',(selected?.targets || []).map(item=>[item.id,item.label]),state.targetId); host.disabled=busy;target.disabled=busy || !selected; - find.disabled=busy;inspect.disabled=busy || !state.targetId;apply.disabled=state.phase!=='ready'; + find.disabled=busy;inspect.disabled=busy || !state.targetId;apply.disabled=busy || !state.targetId || state.phase==='success' || state.outcomeUnknown; preview.replaceChildren(); if(state.preview) { preview.append(element('h3','Review changes')); diff --git a/web/js/workspace/system-integrations.mjs b/web/js/workspace/system-integrations.mjs index 0e2471be..d6632624 100644 --- a/web/js/workspace/system-integrations.mjs +++ b/web/js/workspace/system-integrations.mjs @@ -42,7 +42,7 @@ export const LORA_KEYS = Object.freeze(['lora_enabled', 'lora_trigger_words_enab /** D106: why the options the bridge cannot honour are disabled. */ export const REASONS = Object.freeze({ - send: 'Sending asks when there is more than one ComfyUI window or node and always previews, so “Ask each time” is the only option.', + send: 'Sending asks when there is more than one ComfyUI window or node, so “Ask each time” is the only option.', renderer: 'Prompt Manager nodes always follow ComfyUI’s own renderer setting.', returnCanvas: 'Not available: sending applies the text in ComfyUI and keeps this page open.', });