Merge branch 'main' into add-german-locale

Author: zeus-onl

browser_tests/fixtures/ComfyPage.ts

@@ -645,11 +645,7 @@ export const comfyPageFixture = base.extend<{
         ...(isVueNodes && { 'Comfy.VueNodes.Enabled': true }),
         ...initialSettings
       }
-      try {
-        await comfyPage.setupSettings(startupSettings)
-      } catch (e) {
-        console.error(e)
-      }
+      await comfyPage.setupSettings(startupSettings)
       if (testInfo.tags.includes('@cloud')) {
         const context = page.context()
         await context.route('**/api/auth/session', (route) =>

browser_tests/tests/previewAsText.spec.ts

@@ -55,7 +55,10 @@ test.describe('Preview as Text node', () => {
       await comfyPage.menu.topbar.newWorkflowButton.click()
       await comfyPage.searchBoxV2.addNode('Preview as Text')
       const node = await comfyPage.vueNodes.getFixtureByTitle('Preview as Text')
-      const preview = node.root.locator('textarea')
+      const preview = comfyPage.vueNodes.getWidgetByName(
+        'Preview as Text',
+        'preview_text'
+      )
 
       await test.step('node previews execution result', async () => {
         const id = await comfyPage.vueNodes.getNodeIdByTitle('Preview as Text')
@@ -75,4 +78,59 @@ test.describe('Preview as Text node', () => {
       )
     }
   )
+
+  wstest(
+    'renders the payloads users reported blank',
+    { tag: '@vue-nodes' },
+    async ({ comfyPage, getWebSocket }) => {
+      const execution = new ExecutionHelper(comfyPage, await getWebSocket())
+
+      await comfyPage.menu.topbar.newWorkflowButton.click()
+      await comfyPage.searchBoxV2.addNode('Preview as Text')
+      const preview = comfyPage.vueNodes.getWidgetByName(
+        'Preview as Text',
+        'preview_text'
+      )
+      const id = await comfyPage.vueNodes.getNodeIdByTitle('Preview as Text')
+      const jobId = ''
+
+      const payloads = [
+        ['compact JSON from an LLM node', '{"name":"Comfy","emoji":"🌟"}'],
+        ['JSON array', '[{"a": 1}, {"b": 2}]'],
+        ['markdown-fenced JSON', '```json\n{"name":"Comfy"}\n```'],
+        ['non-ASCII text', '你好，世界。'],
+        ['prompt with a trailing space', '"A red car" is a great prompt. ']
+      ] as const
+
+      for (const [label, text] of payloads) {
+        await test.step(label, async () => {
+          execution.executed(jobId, id, { text: [text] })
+          await expect(preview).toHaveValue(text)
+        })
+      }
+
+      await test.step('numeric output from Get Video Components', async () => {
+        execution.executed(jobId, id, { text: 23.976 })
+        await expect(preview).toHaveValue('23.976')
+      })
+
+      await test.step('null text does not wedge the widget', async () => {
+        // The shape the Cloud backend produced when it misclassified the text
+        // as a filename and dropped it from the payload (BE-3601).
+        execution.executed(jobId, id, { text: [null] })
+        await expect(preview).toHaveValue('')
+
+        execution.executed(jobId, id, { text: ['recovered'] })
+        await expect(preview).toHaveValue('recovered')
+      })
+
+      await test.step('output with no text key does not wedge the widget', async () => {
+        execution.executed(jobId, id, {})
+        await expect(preview).toHaveValue('')
+
+        execution.executed(jobId, id, { text: ['recovered again'] })
+        await expect(preview).toHaveValue('recovered again')
+      })
+    }
+  )
 })

src/extensions/core/textPreviewWidgets.test.ts

@@ -1,6 +1,7 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
+import type { NodeExecutionOutput } from '@/schemas/apiSchema'
 
 interface MockWidget {
   name: string
@@ -100,4 +101,42 @@ describe('updateTextPreviewWidgets', () => {
     updateTextPreviewWidgets(node, { text: 'hello' })
     expect(node.widgets[0].value).toBe('hello')
   })
+
+  it.for([
+    ['null message', null],
+    ['undefined message', undefined],
+    ['message without text', {}],
+    ['null text', { text: null }],
+    ['empty array', { text: [] }],
+    ['array of only nulls', { text: [null, null] }]
+  ] as [string, NodeExecutionOutput | null | undefined][])(
+    'renders empty and does not throw for $0',
+    ([_label, message]) => {
+      expect(() => updateTextPreviewWidgets(node, message)).not.toThrow()
+      expect(node.widgets[0].value).toBe('')
+    }
+  )
+
+  it('drops null entries instead of rendering blank separators', () => {
+    updateTextPreviewWidgets(node, {
+      text: ['first', null, 'second']
+    } as unknown as NodeExecutionOutput)
+
+    expect(node.widgets[0].value).toBe('first\n\nsecond')
+  })
+
+  it('renders non-string entries rather than blanking the node', () => {
+    updateTextPreviewWidgets(node, {
+      text: ['first', 23.976, null, 'second']
+    } as unknown as NodeExecutionOutput)
+
+    expect(node.widgets[0].value).toBe('first\n\n23.976\n\nsecond')
+  })
+
+  it('stringifies a bare non-string scalar payload', () => {
+    updateTextPreviewWidgets(node, {
+      text: 23.976
+    } as unknown as NodeExecutionOutput)
+    expect(node.widgets[0].value).toBe('23.976')
+  })
 })

src/extensions/core/textPreviewWidgets.ts

@@ -1,6 +1,7 @@
 import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 import { resolveNodeRootGraphId } from '@/lib/litegraph/src/litegraph'
 import WidgetTextPreview from '@/renderer/extensions/vueNodes/widgets/components/WidgetTextPreview.vue'
+import type { NodeExecutionOutput } from '@/schemas/apiSchema'
 import type { CustomInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
 import { app } from '@/scripts/app'
 import { ComponentWidgetImpl, addWidget } from '@/scripts/domWidget'
@@ -66,13 +67,19 @@ export function addTextPreviewWidgets(node: LGraphNode) {
   modeWidget.serialize = false
 }
 
+function toPreviewText(text: unknown): string {
+  if (text == null) return ''
+  if (Array.isArray(text))
+    return text.filter((part) => part != null).join('\n\n')
+  return String(text)
+}
+
 export function updateTextPreviewWidgets(
   node: LGraphNode,
-  message: { text?: string | string[] }
+  message: NodeExecutionOutput | null | undefined
 ) {
   const preview = node.widgets?.find((w) => w.name === PREVIEW_WIDGET_NAME)
   if (!preview) return
 
-  const text = message.text ?? ''
-  preview.value = Array.isArray(text) ? text.join('\n\n') : text
+  preview.value = toPreviewText(message?.text)
 }

src/renderer/extensions/firstRunTour/gettingStarted/firstRunEntry.test.ts

@@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type * as VueUseCoreModule from '@vueuse/core'
 
 import type { StartupOutcome } from '@/platform/workflow/persistence/base/draftTypes'
+import type { SharedWorkflowUrlLoadStatus } from '@/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader'
 
 type VueUseCore = typeof VueUseCoreModule
 
@@ -77,6 +78,8 @@ async function freshEntry() {
   return useFirstRunEntry()
 }
 
+type FirstRunEntry = Awaited<ReturnType<typeof freshEntry>>
+
 describe('useFirstRunEntry', () => {
   beforeEach(() => {
     mocks.isCloud = true
@@ -88,8 +91,23 @@ describe('useFirstRunEntry', () => {
     mocks.setSetting.mockImplementation((key: string, value: unknown) => {
       mocks.settings[key] = value
     })
+    // beginTour reports whether a tour actually started; default to the
+    // ordinary case so only tests about a refused start have to say so.
+    mocks.beginTour.mockResolvedValue(true)
   })
 
+  const permanentDisqualifiers = [
+    ['not on cloud', () => void (mocks.isCloud = false)],
+    ['a returning user', () => void (mocks.isNewUser = false)]
+  ] as const
+
+  const transientDisqualifiers = [
+    ['below the md breakpoint', () => void (mocks.isDesktopWidth = false)],
+    ['subscription disabled', () => void (mocks.subscriptionEnabled = false)],
+    ['new-user state undetermined', () => void (mocks.isNewUser = null)],
+    ['the tour flag off', () => void (mocks.tourFlag = false)]
+  ] as const
+
   describe('what a fresh user sees', () => {
     it('shows Getting Started to a candidate', async () => {
       const entry = await freshEntry()
@@ -100,18 +118,6 @@ describe('useFirstRunEntry', () => {
       expect(mocks.execute).not.toHaveBeenCalled()
     })
 
-    const permanentDisqualifiers: [string, () => void][] = [
-      ['not on cloud', () => void (mocks.isCloud = false)],
-      ['a returning user', () => void (mocks.isNewUser = false)]
-    ]
-
-    const transientDisqualifiers: [string, () => void][] = [
-      ['below the md breakpoint', () => void (mocks.isDesktopWidth = false)],
-      ['subscription disabled', () => void (mocks.subscriptionEnabled = false)],
-      ['new-user state undetermined', () => void (mocks.isNewUser = null)],
-      ['the tour flag off', () => void (mocks.tourFlag = false)]
-    ]
-
     it.for([...permanentDisqualifiers, ...transientDisqualifiers])(
       'opens the template browser instead, with %s',
       async ([, disqualify]) => {
@@ -174,7 +180,7 @@ describe('useFirstRunEntry', () => {
     })
   })
 
-  it('marks a url-intent startup completed without taking over the screen', async () => {
+  it('marks a url-intent startup completed once its tour starts, without taking over the screen', async () => {
     const entry = await freshEntry()
 
     await entry.handleStartupOutcome('url-intent')
@@ -184,12 +190,86 @@ describe('useFirstRunEntry', () => {
       'A share or template link is the user’s choice; onboarding must not cover it'
     ).toBe(false)
     expect(mocks.execute).not.toHaveBeenCalled()
+
+    await entry.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+
     expect(
       mocks.setSetting,
       'Without this the template browser reopens on every launch, as it did before this flow existed'
     ).toHaveBeenCalledWith('Comfy.TutorialCompleted', true)
   })
 
+  /** Both shapes of URL a first-run boot can arrive on. */
+  const urlArrivals = [
+    ['a template link', 'image_z_image_turbo', undefined],
+    ['a share link', undefined, 'loaded']
+  ] as const satisfies readonly [
+    string,
+    string | undefined,
+    SharedWorkflowUrlLoadStatus | undefined
+  ][]
+
+  const deferredUrlBoots = transientDisqualifiers.flatMap(([why, disqualify]) =>
+    urlArrivals.map(
+      ([arrival, templateId, sharedStatus]) =>
+        [
+          `${arrival} with ${why}`,
+          disqualify,
+          templateId,
+          sharedStatus
+        ] as const
+    )
+  )
+
+  it.for(deferredUrlBoots)(
+    'runs a url-intent boot to the end for %s, offering no tour and keeping eligibility',
+    async ([, disqualify, templateId, sharedStatus]) => {
+      disqualify()
+      const entry = await freshEntry()
+
+      await entry.handleStartupOutcome('url-intent')
+      await entry.handleUrlWorkflow('url-intent', templateId, sharedStatus)
+
+      expect(
+        mocks.beginTour,
+        'an ineligible boot has no tour to give'
+      ).not.toHaveBeenCalled()
+      expect(
+        entry.gettingStartedVisible.value,
+        'the link is the user’s choice; onboarding must not cover it'
+      ).toBe(false)
+      expect(
+        mocks.setSetting,
+        'no tour ran, so the write-once flag that pays for one must stay unspent for the boot that can lift this'
+      ).not.toHaveBeenCalled()
+    }
+  )
+
+  it('tours a template link on the boot after one that only deferred', async () => {
+    mocks.isDesktopWidth = false
+    const phone = await freshEntry()
+    await phone.handleStartupOutcome('url-intent')
+    await phone.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+
+    expect(mocks.beginTour).not.toHaveBeenCalled()
+
+    mocks.isDesktopWidth = true
+    // What `checkIsNewUser()` reads on the next launch.
+    mocks.isNewUser = !mocks.settings['Comfy.TutorialCompleted']
+    const laptop = await freshEntry()
+    await laptop.handleStartupOutcome('url-intent')
+    await laptop.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+
+    expect(
+      mocks.beginTour,
+      'opening a template link on a phone and returning on a laptop is ordinary behaviour'
+    ).toHaveBeenCalledWith('image_z_image_turbo')
+    expect(
+      mocks.settings['Comfy.TutorialCompleted'],
+      'the flag is spent on the boot that finally delivered the tour, not before'
+    ).toBe(true)
+  })
+
   it('never onboards over restored work, even for an apparent new user', async () => {
     const entry = await freshEntry()
 
@@ -205,18 +285,34 @@ describe('useFirstRunEntry', () => {
     ).not.toHaveBeenCalled()
   })
 
-  it.for(['fresh', 'url-intent'] as const)(
+  /**
+   * A `url-intent` boot only settles once `handleUrlWorkflow` has seen what the
+   * link loaded, so the invariant is asserted over the pair of handlers that
+   * GraphCanvas awaits in turn, not over the first one alone.
+   */
+  const startups: [StartupOutcome, (entry: FirstRunEntry) => Promise<void>][] =
+    [
+      ['fresh', async () => {}],
+      [
+        'url-intent',
+        (entry) => entry.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+      ]
+    ]
+
+  it.for(startups)(
     'settles the first-run decision on a %s startup rather than leaving it pending',
-    async (outcome: StartupOutcome) => {
+    async ([outcome, finishBoot]) => {
       const entry = await freshEntry()
 
       await entry.handleStartupOutcome(outcome)
+      await finishBoot(entry)
 
       expect(
         entry.gettingStartedVisible.value ||
+          mocks.beginTour.mock.calls.length > 0 ||
           mocks.setSetting.mock.calls.length > 0 ||
           mocks.execute.mock.calls.length > 0,
-        'a startup that opened a blank canvas must offer onboarding, record that it is done, or open the template browser; anything else strands the user with an empty screen'
+        'a startup that opened a blank canvas must offer onboarding, tour what the link loaded, record that it is done, or open the template browser; anything else strands the user with an empty screen'
       ).toBe(true)
     }
   )
@@ -259,7 +355,7 @@ describe('useFirstRunEntry', () => {
       ).toHaveBeenCalledWith(undefined)
     })
 
-    it('leaves the completion flag to the startup handler that already settled it', async () => {
+    it('leaves the completion flag alone when the engine refused to start', async () => {
       const entry = await freshEntry()
       await entry.handleStartupOutcome('url-intent')
       mocks.setSetting.mockClear()
@@ -269,7 +365,23 @@ describe('useFirstRunEntry', () => {
 
       expect(
         mocks.setSetting,
-        'writing it again here would mark onboarding done for a user whose tour never started, and postpone() exists to offer that user the tour again'
+        'writing it here would mark onboarding done for a user whose tour never started, and postpone() exists to offer that user the tour again'
+      ).not.toHaveBeenCalled()
+    })
+
+    it('keeps the account eligible when the link loaded nothing to tour', async () => {
+      const entry = await freshEntry()
+
+      await entry.handleStartupOutcome('url-intent')
+      await entry.handleUrlWorkflow('url-intent', undefined, 'failed')
+
+      expect(
+        mocks.beginTour,
+        'there is no workflow on the canvas to tour'
+      ).not.toHaveBeenCalled()
+      expect(
+        mocks.setSetting,
+        'a dead link must not spend the one tour the account gets; the next boot has no URL to honour and offers Getting Started instead'
       ).not.toHaveBeenCalled()
     })