From 2ab1abb898d22bfbf286b1ebb4e1889389db040f Mon Sep 17 00:00:00 2001
From: Deep Mehta <42841935+deepme987@users.noreply.github.com>
Date: Tue, 19 May 2026 15:58:12 -0700
Subject: [PATCH] Revert "fix(cloud): stop bouncing working users to
 /cloud/survey mid-session" (#12344)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reverts Comfy-Org/ComfyUI_frontend#12301

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12344-Revert-fix-cloud-stop-bouncing-working-users-to-cloud-survey-mid-session-3656d73d365081119ebad749a2e0d403)
by [Unito](https://www.unito.io)
---
 src/platform/cloud/onboarding/auth.test.ts | 91 ----------------------
 src/platform/cloud/onboarding/auth.ts      | 13 ++--
 2 files changed, 6 insertions(+), 98 deletions(-)
 delete mode 100644 src/platform/cloud/onboarding/auth.test.ts

diff --git a/src/platform/cloud/onboarding/auth.test.ts b/src/platform/cloud/onboarding/auth.test.ts
deleted file mode 100644
index 40d2cd1fcc..0000000000
--- a/src/platform/cloud/onboarding/auth.test.ts
+++ /dev/null
@@ -1,91 +0,0 @@
-import { fromPartial } from '@total-typescript/shoehorn'
-import { beforeEach, describe, expect, test, vi } from 'vitest'
-
-import { getSurveyCompletedStatus } from './auth'
-
-const fetchApi = vi.fn()
-
-vi.mock('@/scripts/api', () => ({
-  api: {
-    fetchApi: (...args: unknown[]) => fetchApi(...args)
-  }
-}))
-
-vi.mock('@sentry/vue', () => ({
-  addBreadcrumb: vi.fn(),
-  captureException: vi.fn()
-}))
-
-function mockResponse({
-  ok,
-  status,
-  body
-}: {
-  ok: boolean
-  status: number
-  body?: unknown
-}): Response {
-  return fromPartial<Response>({
-    ok,
-    status,
-    statusText: '',
-    json: async () => body
-  })
-}
-
-describe('getSurveyCompletedStatus', () => {
-  beforeEach(() => {
-    fetchApi.mockReset()
-  })
-
-  test('200 with non-empty value → true', async () => {
-    fetchApi.mockResolvedValueOnce(
-      mockResponse({ ok: true, status: 200, body: { value: { q1: 'a' } } })
-    )
-    await expect(getSurveyCompletedStatus()).resolves.toBe(true)
-  })
-
-  test('200 with empty value → false (the only "not completed" signal)', async () => {
-    fetchApi.mockResolvedValueOnce(
-      mockResponse({ ok: true, status: 200, body: { value: {} } })
-    )
-    await expect(getSurveyCompletedStatus()).resolves.toBe(false)
-  })
-
-  test('200 with null value → false', async () => {
-    fetchApi.mockResolvedValueOnce(
-      mockResponse({ ok: true, status: 200, body: { value: null } })
-    )
-    await expect(getSurveyCompletedStatus()).resolves.toBe(false)
-  })
-
-  test('404 → true (do not bounce on missing key)', async () => {
-    fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 404 }))
-    await expect(getSurveyCompletedStatus()).resolves.toBe(true)
-  })
-
-  test('500 → true (do not bounce on transient backend error)', async () => {
-    fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 500 }))
-    await expect(getSurveyCompletedStatus()).resolves.toBe(true)
-  })
-
-  // 401/403 fall under the same "ambiguous => treat as completed" policy.
-  // The dedicated auth layer handles re-authentication on the next API
-  // call; this function deliberately does not try to disambiguate auth
-  // failures from other non-OK responses. Locking with tests so the
-  // policy can't drift back to a "throw on auth error" branch.
-  test('401 → true (auth layer handles re-auth on next call)', async () => {
-    fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 401 }))
-    await expect(getSurveyCompletedStatus()).resolves.toBe(true)
-  })
-
-  test('403 → true (auth layer handles re-auth on next call)', async () => {
-    fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 403 }))
-    await expect(getSurveyCompletedStatus()).resolves.toBe(true)
-  })
-
-  test('network rejection → true (do not bounce on network error)', async () => {
-    fetchApi.mockRejectedValueOnce(new TypeError('Network request failed'))
-    await expect(getSurveyCompletedStatus()).resolves.toBe(true)
-  })
-})
diff --git a/src/platform/cloud/onboarding/auth.ts b/src/platform/cloud/onboarding/auth.ts
index 31ef70d726..30a1d7878e 100644
--- a/src/platform/cloud/onboarding/auth.ts
+++ b/src/platform/cloud/onboarding/auth.ts
@@ -96,24 +96,23 @@ export async function getSurveyCompletedStatus(): Promise<boolean> {
       }
     })
     if (!response.ok) {
-      // Ambiguous response (404/5xx/etc). Treat as completed to avoid
-      // bouncing working customers to /cloud/survey on transient hiccups.
-      // Real "not completed" only comes from a 200 with empty value.
+      // Not an error case - survey not completed is a valid state
       Sentry.addBreadcrumb({
         category: 'auth',
         message: 'Survey status check returned non-ok response',
-        level: 'warning',
+        level: 'info',
         data: {
           status: response.status,
           endpoint: `/settings/${ONBOARDING_SURVEY_KEY}`
         }
       })
-      return true
+      return false
     }
     const data = await response.json()
+    // Check if data exists and is not empty
     return !isEmpty(data.value)
   } catch (error) {
-    // Network/parse failure — same policy as ambiguous HTTP responses.
+    // Network error - still capture it as it's not thrown from above
     Sentry.captureException(error, {
       tags: {
         api_endpoint: '/settings/{key}',
@@ -125,7 +124,7 @@ export async function getSurveyCompletedStatus(): Promise<boolean> {
       },
       level: 'warning'
     })
-    return true
+    return false
   }
 }