fix(api): release non-stream plugin chunk iterator

AGENTS.md

@@ -25,30 +25,6 @@ pnpm type-check:tsgo
 pnpm test
 ```
 
-### Frontend Linting
-
-ESLint is used for frontend code quality. Available commands:
-
-```bash
-# Lint all files (report only)
-pnpm lint
-
-# Lint and auto-fix issues
-pnpm lint:fix
-
-# Lint specific files or directories
-pnpm lint:fix app/components/base/button/
-pnpm lint:fix app/components/base/button/index.tsx
-
-# Lint quietly (errors only, no warnings)
-pnpm lint:quiet
-
-# Check code complexity
-pnpm lint:complexity
-```
-
-**Important**: Always run `pnpm lint:fix` before committing. The pre-commit hook runs `lint-staged` which only lints staged files.
-
 ## Testing & Quality Practices
 
 - Follow TDD: red → green → refactor.

agent-notes/api/core/model_runtime/model_providers/__base/large_language_model.py.md

@@ -0,0 +1,27 @@
+# Notes: `large_language_model.py`
+
+## Purpose
+
+Provides the base `LargeLanguageModel` implementation used by the model runtime to invoke plugin-backed LLMs and to
+bridge plugin daemon streaming semantics back into API-layer entities (`LLMResult`, `LLMResultChunk`).
+
+## Key behaviors / invariants
+
+- `invoke(..., stream=False)` still calls the plugin in streaming mode and then synthesizes a single `LLMResult` from
+  the first yielded `LLMResultChunk`.
+- Plugin invocation is wrapped by `_invoke_llm_via_plugin(...)`, and `stream=False` normalization is handled by
+  `_normalize_non_stream_plugin_result(...)` / `_build_llm_result_from_first_chunk(...)`.
+- Tool call deltas are merged incrementally via `_increase_tool_call(...)` to support multiple provider chunking
+  patterns (IDs anchored to first chunk, every chunk, or missing entirely).
+- A tool-call delta with an empty `id` requires at least one existing tool call; otherwise we raise `ValueError` to
+  surface invalid delta sequences explicitly.
+- Callback invocation is centralized in `_run_callbacks(...)` to ensure consistent error handling/logging.
+- For compatibility with dify issue `#17799`, `prompt_messages` may be removed by the plugin daemon in chunks and must
+  be re-attached in this layer before callbacks/consumers use them.
+- Callback hooks (`on_before_invoke`, `on_new_chunk`, `on_after_invoke`, `on_invoke_error`) must not break invocation
+  unless `callback.raise_error` is true.
+
+## Test focus
+
+- `api/tests/unit_tests/core/model_runtime/__base/test_increase_tool_call.py` validates tool-call delta merging and
+  patches `_gen_tool_call_id` for deterministic IDs.

api/AGENTS.md

@@ -1,47 +1,97 @@
 # API Agent Guide
 
-## Notes for Agent (must-check)
+## Agent Notes (must-check)
 
-Before changing any backend code under `api/`, you MUST read the surrounding docstrings and comments. These notes contain required context (invariants, edge cases, trade-offs) and are treated as part of the spec.
+Before you start work on any backend file under `api/`, you MUST check whether a related note exists under:
 
-Look for:
+- `agent-notes/<same-relative-path-as-target-file>.md`
 
-- The module (file) docstring at the top of a source code file
-- Docstrings on classes and functions/methods
-- Paragraph/block comments for non-obvious logic
+Rules:
 
-### What to write where
+- **Path mapping**: for a target file `<path>/<name>.py`, the note must be `agent-notes/<path>/<name>.py.md` (same folder structure, same filename, plus `.md`).
+- **Before working**:
+  - If the note exists, read it first and follow any constraints/decisions recorded there.
+  - If the note conflicts with the current code, or references an "origin" file/path that has been deleted, renamed, or migrated, treat the **code as the single source of truth** and update the note to match reality.
+  - If the note does not exist, create it with a short architecture/intent summary and any relevant invariants/edge cases.
+- **During working**:
+  - Keep the note in sync as you discover constraints, make decisions, or change approach.
+  - If you move/rename a file, migrate its note to the new mapped path (and fix any outdated references inside the note).
+  - Record non-obvious edge cases, trade-offs, and the test/verification plan as you go (not just at the end).
+  - Keep notes **coherent**: integrate new findings into the relevant sections and rewrite for clarity; avoid append-only “recent fix” / changelog-style additions unless the note is explicitly intended to be a changelog.
+- **When finishing work**:
+  - Update the related note(s) to reflect what changed, why, and any new edge cases/tests.
+  - If a file is deleted, remove or clearly deprecate the corresponding note so it cannot be mistaken as current guidance.
+  - Keep notes concise and accurate; they are meant to prevent repeated rediscovery.
 
-- Keep notes scoped: module notes cover module-wide context, class notes cover class-wide context, function/method notes cover behavioural contracts, and paragraph/block comments cover local “why”. Avoid duplicating the same content across scopes unless repetition prevents misuse.
-- **Module (file) docstring**: purpose, boundaries, key invariants, and “gotchas” that a new reader must know before editing.
-  - Include cross-links to the key collaborators (modules/services) when discovery is otherwise hard.
-  - Prefer stable facts (invariants, contracts) over ephemeral “today we…” notes.
-- **Class docstring**: responsibility, lifecycle, invariants, and how it should be used (or not used).
-  - If the class is intentionally stateful, note what state exists and what methods mutate it.
-  - If concurrency/async assumptions matter, state them explicitly.
-- **Function/method docstring**: behavioural contract.
-  - Document arguments, return shape, side effects (DB writes, external I/O, task dispatch), and raised domain exceptions.
-  - Add examples only when they prevent misuse.
-- **Paragraph/block comments**: explain *why* (trade-offs, historical constraints, surprising edge cases), not what the code already states.
-  - Keep comments adjacent to the logic they justify; delete or rewrite comments that no longer match reality.
+## Skill Index
 
-### Rules (must follow)
+Start with the section that best matches your need. Each entry lists the problems it solves plus key files/concepts so you know what to expect before opening it.
 
-In this section, “notes” means module/class/function docstrings plus any relevant paragraph/block comments.
+### Platform Foundations
 
-- **Before working**
-  - Read the notes in the area you’ll touch; treat them as part of the spec.
-  - If a docstring or comment conflicts with the current code, treat the **code as the single source of truth** and update the docstring or comment to match reality.
-  - If important intent/invariants/edge cases are missing, add them in the closest docstring or comment (module for overall scope, function for behaviour).
-- **During working**
-  - Keep the notes in sync as you discover constraints, make decisions, or change approach.
-  - If you move/rename responsibilities across modules/classes, update the affected docstrings and comments so readers can still find the “why” and the invariants.
-  - Record non-obvious edge cases, trade-offs, and the test/verification plan in the nearest docstring or comment that will stay correct.
-  - Keep the notes **coherent**: integrate new findings into the relevant docstrings and comments; avoid append-only “recent fix” / changelog-style additions.
-- **When finishing**
-  - Update the notes to reflect what changed, why, and any new edge cases/tests.
-  - Remove or rewrite any comments that could be mistaken as current guidance but no longer apply.
-  - Keep docstrings and comments concise and accurate; they are meant to prevent repeated rediscovery.
+#### [Infrastructure Overview](agent_skills/infra.md)
+
+- **When to read this**
+  - You need to understand where a feature belongs in the architecture.
+  - You’re wiring storage, Redis, vector stores, or OTEL.
+  - You’re about to add CLI commands or async jobs.
+- **What it covers**
+  - Configuration stack (`configs/app_config.py`, remote settings)
+  - Storage entry points (`extensions/ext_storage.py`, `core/file/file_manager.py`)
+  - Redis conventions (`extensions/ext_redis.py`)
+  - Plugin runtime topology
+  - Vector-store factory (`core/rag/datasource/vdb/*`)
+  - Observability hooks
+  - SSRF proxy usage
+  - Core CLI commands
+
+### Plugin & Extension Development
+
+#### [Plugin Systems](agent_skills/plugin.md)
+
+- **When to read this**
+  - You’re building or debugging a marketplace plugin.
+  - You need to know how manifests, providers, daemons, and migrations fit together.
+- **What it covers**
+  - Plugin manifests (`core/plugin/entities/plugin.py`)
+  - Installation/upgrade flows (`services/plugin/plugin_service.py`, CLI commands)
+  - Runtime adapters (`core/plugin/impl/*` for tool/model/datasource/trigger/endpoint/agent)
+  - Daemon coordination (`core/plugin/entities/plugin_daemon.py`)
+  - How provider registries surface capabilities to the rest of the platform
+
+#### [Plugin OAuth](agent_skills/plugin_oauth.md)
+
+- **When to read this**
+  - You must integrate OAuth for a plugin or datasource.
+  - You’re handling credential encryption or refresh flows.
+- **Topics**
+  - Credential storage
+  - Encryption helpers (`core/helper/provider_encryption.py`)
+  - OAuth client bootstrap (`services/plugin/oauth_service.py`, `services/plugin/plugin_parameter_service.py`)
+  - How console/API layers expose the flows
+
+### Workflow Entry & Execution
+
+#### [Trigger Concepts](agent_skills/trigger.md)
+
+- **When to read this**
+  - You’re debugging why a workflow didn’t start.
+  - You’re adding a new trigger type or hook.
+  - You need to trace async execution, draft debugging, or webhook/schedule pipelines.
+- **Details**
+  - Start-node taxonomy
+  - Webhook & schedule internals (`core/workflow/nodes/trigger_*`, `services/trigger/*`)
+  - Async orchestration (`services/async_workflow_service.py`, Celery queues)
+  - Debug event bus
+  - Storage/logging interactions
+
+## General Reminders
+
+- All skill docs assume you follow the coding style rules below—run the lint/type/test commands before submitting changes.
+- When you cannot find an answer in these briefs, search the codebase using the paths referenced (e.g., `core/plugin/impl/tool.py`, `services/dataset_service.py`).
+- If you run into cross-cutting concerns (tenancy, configuration, storage), check the infrastructure guide first; it links to most supporting modules.
+- Keep multi-tenancy and configuration central: everything flows through `configs.dify_config` and `tenant_id`.
+- When touching plugins or triggers, consult both the system overview and the specialised doc to ensure you adjust lifecycle, storage, and observability consistently.
 
 ## Coding Style
 
@@ -176,7 +226,7 @@ Before opening a PR / submitting:
 
 - Controllers: parse input via Pydantic, invoke services, return serialised responses; no business logic.
 - Services: coordinate repositories, providers, background tasks; keep side effects explicit.
-- Document non-obvious behaviour with concise docstrings and comments.
+- Document non-obvious behaviour with concise comments.
 
 ### Miscellaneous
 

api/core/model_runtime/model_providers/__base/large_language_model.py

@@ -92,6 +92,10 @@ def _build_llm_result_from_first_chunk(
     Build a single `LLMResult` from the first returned chunk.
 
     This is used for `stream=False` because the plugin side may still implement the response via a chunked stream.
+
+    Note:
+        This function always drains the `chunks` iterator after reading the first chunk to ensure any underlying
+        streaming resources are released (e.g., HTTP connections owned by the plugin runtime).
     """
     content = ""
     content_list: list[PromptMessageContentUnionTypes] = []
@@ -99,18 +103,25 @@ def _build_llm_result_from_first_chunk(
     system_fingerprint: str | None = None
     tools_calls: list[AssistantPromptMessage.ToolCall] = []
 
-    first_chunk = next(chunks, None)
-    if first_chunk is not None:
-        if isinstance(first_chunk.delta.message.content, str):
-            content += first_chunk.delta.message.content
-        elif isinstance(first_chunk.delta.message.content, list):
-            content_list.extend(first_chunk.delta.message.content)
+    try:
+        first_chunk = next(chunks, None)
+        if first_chunk is not None:
+            if isinstance(first_chunk.delta.message.content, str):
+                content += first_chunk.delta.message.content
+            elif isinstance(first_chunk.delta.message.content, list):
+                content_list.extend(first_chunk.delta.message.content)
 
-        if first_chunk.delta.message.tool_calls:
-            _increase_tool_call(first_chunk.delta.message.tool_calls, tools_calls)
+            if first_chunk.delta.message.tool_calls:
+                _increase_tool_call(first_chunk.delta.message.tool_calls, tools_calls)
 
-        usage = first_chunk.delta.usage or LLMUsage.empty_usage()
-        system_fingerprint = first_chunk.system_fingerprint
+            usage = first_chunk.delta.usage or LLMUsage.empty_usage()
+            system_fingerprint = first_chunk.system_fingerprint
+    finally:
+        try:
+            for _ in chunks:
+                pass
+        except Exception:
+            logger.debug("Failed to drain non-stream plugin chunk iterator.", exc_info=True)
 
     return LLMResult(
         model=model,

api/tests/unit_tests/core/model_runtime/__base/test_large_language_model_non_stream_parsing.py

@@ -101,3 +101,26 @@ def test__normalize_non_stream_plugin_result__empty_iterator_defaults():
     assert result.message.tool_calls == []
     assert result.usage == LLMUsage.empty_usage()
     assert result.system_fingerprint is None
+
+
+def test__normalize_non_stream_plugin_result__closes_chunk_iterator():
+    prompt_messages = [UserPromptMessage(content="hi")]
+
+    chunk = _make_chunk(content="hello", usage=LLMUsage.empty_usage())
+    closed: list[bool] = []
+
+    def _chunk_iter():
+        try:
+            yield chunk
+            yield _make_chunk(content="ignored", usage=LLMUsage.empty_usage())
+        finally:
+            closed.append(True)
+
+    result = _normalize_non_stream_plugin_result(
+        model="test-model",
+        prompt_messages=prompt_messages,
+        result=_chunk_iter(),
+    )
+
+    assert result.message.content == "hello"
+    assert closed == [True]

web/app/components/workflow-app/hooks/use-nodes-sync-draft.spec.ts

@@ -1,705 +0,0 @@
-/**
- * Test Suite for useNodesSyncDraft Hook
- *
- * PURPOSE:
- * This hook handles syncing workflow draft to the server. The key fix being tested
- * is the error handling behavior when `draft_workflow_not_sync` error occurs.
- *
- * MULTI-TAB PROBLEM SCENARIO:
- * 1. User opens the same workflow in Tab A and Tab B (both have hash: v1)
- * 2. Tab A saves successfully, server returns new hash: v2
- * 3. Tab B tries to save with old hash: v1, server returns 400 error with code
- *    'draft_workflow_not_sync'
- * 4. BEFORE FIX: handleRefreshWorkflowDraft() was called without args, which fetched
- *    draft AND overwrote canvas - user lost unsaved changes in Tab B
- * 5. AFTER FIX: handleRefreshWorkflowDraft(true) is called, which fetches draft but
- *    only updates hash (notUpdateCanvas=true), preserving user's canvas changes
- *
- * TESTING STRATEGY:
- * We don't simulate actual tab switching UI behavior. Instead, we mock the API to
- * return `draft_workflow_not_sync` error and verify:
- * - The hook calls handleRefreshWorkflowDraft(true) - not handleRefreshWorkflowDraft()
- * - This ensures canvas data is preserved while hash is updated for retry
- *
- * This is behavior-driven testing - we verify "what the code does when receiving
- * specific API errors" rather than simulating complete user interaction flows.
- * True multi-tab integration testing would require E2E frameworks like Playwright.
- */
-
-import { act, renderHook, waitFor } from '@testing-library/react'
-import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { useNodesSyncDraft } from './use-nodes-sync-draft'
-
-// Mock reactflow store
-const mockGetNodes = vi.fn()
-
-type MockEdge = {
-  id: string
-  source: string
-  target: string
-  data: Record<string, unknown>
-}
-
-const mockStoreState: {
-  getNodes: ReturnType<typeof vi.fn>
-  edges: MockEdge[]
-  transform: number[]
-} = {
-  getNodes: mockGetNodes,
-  edges: [],
-  transform: [0, 0, 1],
-}
-vi.mock('reactflow', () => ({
-  useStoreApi: () => ({
-    getState: () => mockStoreState,
-  }),
-}))
-
-// Mock features store
-const mockFeaturesState = {
-  features: {
-    opening: { enabled: false, opening_statement: '', suggested_questions: [] },
-    suggested: {},
-    text2speech: {},
-    speech2text: {},
-    citation: {},
-    moderation: {},
-    file: {},
-  },
-}
-vi.mock('@/app/components/base/features/hooks', () => ({
-  useFeaturesStore: () => ({
-    getState: () => mockFeaturesState,
-  }),
-}))
-
-// Mock workflow service
-const mockSyncWorkflowDraft = vi.fn()
-vi.mock('@/service/workflow', () => ({
-  syncWorkflowDraft: (...args: unknown[]) => mockSyncWorkflowDraft(...args),
-}))
-
-// Mock useNodesReadOnly
-const mockGetNodesReadOnly = vi.fn()
-vi.mock('@/app/components/workflow/hooks/use-workflow', () => ({
-  useNodesReadOnly: () => ({
-    getNodesReadOnly: mockGetNodesReadOnly,
-  }),
-}))
-
-// Mock useSerialAsyncCallback - pass through the callback
-vi.mock('@/app/components/workflow/hooks/use-serial-async-callback', () => ({
-  useSerialAsyncCallback: (callback: (...args: unknown[]) => unknown) => callback,
-}))
-
-// Mock workflow store
-const mockSetSyncWorkflowDraftHash = vi.fn()
-const mockSetDraftUpdatedAt = vi.fn()
-
-const createMockWorkflowStoreState = (overrides = {}) => ({
-  appId: 'test-app-id',
-  conversationVariables: [],
-  environmentVariables: [],
-  syncWorkflowDraftHash: 'current-hash-123',
-  isWorkflowDataLoaded: true,
-  setSyncWorkflowDraftHash: mockSetSyncWorkflowDraftHash,
-  setDraftUpdatedAt: mockSetDraftUpdatedAt,
-  ...overrides,
-})
-
-const mockWorkflowStoreGetState = vi.fn()
-vi.mock('@/app/components/workflow/store', () => ({
-  useWorkflowStore: () => ({
-    getState: mockWorkflowStoreGetState,
-  }),
-}))
-
-// Mock useWorkflowRefreshDraft (THE KEY DEPENDENCY FOR THIS TEST)
-const mockHandleRefreshWorkflowDraft = vi.fn()
-vi.mock('.', () => ({
-  useWorkflowRefreshDraft: () => ({
-    handleRefreshWorkflowDraft: mockHandleRefreshWorkflowDraft,
-  }),
-}))
-
-// Mock API_PREFIX
-vi.mock('@/config', () => ({
-  API_PREFIX: '/api',
-}))
-
-// Create a mock error response that mimics the actual API error
-const createMockErrorResponse = (code: string) => {
-  const errorBody = { code, message: 'Draft not in sync' }
-  let bodyUsed = false
-
-  return {
-    json: vi.fn().mockImplementation(() => {
-      bodyUsed = true
-      return Promise.resolve(errorBody)
-    }),
-    get bodyUsed() {
-      return bodyUsed
-    },
-  }
-}
-
-describe('useNodesSyncDraft', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-    mockGetNodesReadOnly.mockReturnValue(false)
-    mockGetNodes.mockReturnValue([
-      { id: 'node-1', type: 'start', data: { type: 'start' } },
-      { id: 'node-2', type: 'llm', data: { type: 'llm' } },
-    ])
-    mockStoreState.edges = [
-      { id: 'edge-1', source: 'node-1', target: 'node-2', data: {} },
-    ]
-    mockWorkflowStoreGetState.mockReturnValue(createMockWorkflowStoreState())
-    mockSyncWorkflowDraft.mockResolvedValue({
-      hash: 'new-hash-456',
-      updated_at: Date.now(),
-    })
-  })
-
-  afterEach(() => {
-    vi.resetAllMocks()
-  })
-
-  describe('doSyncWorkflowDraft function', () => {
-    it('should return doSyncWorkflowDraft function', () => {
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      expect(result.current.doSyncWorkflowDraft).toBeDefined()
-      expect(typeof result.current.doSyncWorkflowDraft).toBe('function')
-    })
-
-    it('should return syncWorkflowDraftWhenPageClose function', () => {
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      expect(result.current.syncWorkflowDraftWhenPageClose).toBeDefined()
-      expect(typeof result.current.syncWorkflowDraftWhenPageClose).toBe('function')
-    })
-  })
-
-  describe('successful sync', () => {
-    it('should call syncWorkflowDraft service on successful sync', async () => {
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSyncWorkflowDraft).toHaveBeenCalledWith({
-        url: '/apps/test-app-id/workflows/draft',
-        params: expect.objectContaining({
-          hash: 'current-hash-123',
-          graph: expect.objectContaining({
-            nodes: expect.any(Array),
-            edges: expect.any(Array),
-            viewport: expect.any(Object),
-          }),
-        }),
-      })
-    })
-
-    it('should update syncWorkflowDraftHash on success', async () => {
-      mockSyncWorkflowDraft.mockResolvedValue({
-        hash: 'new-hash-789',
-        updated_at: 1234567890,
-      })
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-789')
-    })
-
-    it('should update draftUpdatedAt on success', async () => {
-      const updatedAt = 1234567890
-      mockSyncWorkflowDraft.mockResolvedValue({
-        hash: 'new-hash',
-        updated_at: updatedAt,
-      })
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSetDraftUpdatedAt).toHaveBeenCalledWith(updatedAt)
-    })
-
-    it('should call onSuccess callback on success', async () => {
-      const onSuccess = vi.fn()
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onSuccess })
-      })
-
-      expect(onSuccess).toHaveBeenCalled()
-    })
-
-    it('should call onSettled callback after success', async () => {
-      const onSettled = vi.fn()
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onSettled })
-      })
-
-      expect(onSettled).toHaveBeenCalled()
-    })
-  })
-
-  describe('sync error handling - draft_workflow_not_sync (THE KEY FIX)', () => {
-    /**
-     * This is THE KEY TEST for the bug fix.
-     *
-     * SCENARIO: Multi-tab editing
-     * 1. User opens workflow in Tab A and Tab B
-     * 2. Tab A saves draft successfully, gets new hash
-     * 3. Tab B tries to save with old hash
-     * 4. Server returns 400 with code 'draft_workflow_not_sync'
-     *
-     * BEFORE FIX:
-     * - handleRefreshWorkflowDraft() was called without arguments
-     * - This would fetch draft AND overwrite the canvas
-     * - User loses their unsaved changes in Tab B
-     *
-     * AFTER FIX:
-     * - handleRefreshWorkflowDraft(true) is called
-     * - This fetches draft but DOES NOT overwrite canvas
-     * - Only hash is updated for the next sync attempt
-     * - User's unsaved changes are preserved
-     */
-    it('should call handleRefreshWorkflowDraft with notUpdateCanvas=true when draft_workflow_not_sync error occurs', async () => {
-      const mockError = createMockErrorResponse('draft_workflow_not_sync')
-      mockSyncWorkflowDraft.mockRejectedValue(mockError)
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      // THE KEY ASSERTION: handleRefreshWorkflowDraft must be called with true
-      await waitFor(() => {
-        expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledWith(true)
-      })
-    })
-
-    it('should NOT call handleRefreshWorkflowDraft when notRefreshWhenSyncError is true', async () => {
-      const mockError = createMockErrorResponse('draft_workflow_not_sync')
-      mockSyncWorkflowDraft.mockRejectedValue(mockError)
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        // First parameter is notRefreshWhenSyncError
-        await result.current.doSyncWorkflowDraft(true)
-      })
-
-      // Wait a bit for async operations
-      await new Promise(resolve => setTimeout(resolve, 100))
-
-      expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled()
-    })
-
-    it('should call onError callback when draft_workflow_not_sync error occurs', async () => {
-      const mockError = createMockErrorResponse('draft_workflow_not_sync')
-      mockSyncWorkflowDraft.mockRejectedValue(mockError)
-      const onError = vi.fn()
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onError })
-      })
-
-      expect(onError).toHaveBeenCalled()
-    })
-
-    it('should call onSettled callback after error', async () => {
-      const mockError = createMockErrorResponse('draft_workflow_not_sync')
-      mockSyncWorkflowDraft.mockRejectedValue(mockError)
-      const onSettled = vi.fn()
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onSettled })
-      })
-
-      expect(onSettled).toHaveBeenCalled()
-    })
-  })
-
-  describe('other error handling', () => {
-    it('should NOT call handleRefreshWorkflowDraft for non-draft_workflow_not_sync errors', async () => {
-      const mockError = createMockErrorResponse('some_other_error')
-      mockSyncWorkflowDraft.mockRejectedValue(mockError)
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      // Wait a bit for async operations
-      await new Promise(resolve => setTimeout(resolve, 100))
-
-      expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled()
-    })
-
-    it('should handle error without json method', async () => {
-      const mockError = new Error('Network error')
-      mockSyncWorkflowDraft.mockRejectedValue(mockError)
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-      const onError = vi.fn()
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onError })
-      })
-
-      expect(onError).toHaveBeenCalled()
-      expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled()
-    })
-
-    it('should handle error with bodyUsed already true', async () => {
-      const mockError = {
-        json: vi.fn(),
-        bodyUsed: true,
-      }
-      mockSyncWorkflowDraft.mockRejectedValue(mockError)
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      // Should not call json() when bodyUsed is true
-      expect(mockError.json).not.toHaveBeenCalled()
-      expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled()
-    })
-  })
-
-  describe('read-only mode', () => {
-    it('should not sync when nodes are read-only', async () => {
-      mockGetNodesReadOnly.mockReturnValue(true)
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSyncWorkflowDraft).not.toHaveBeenCalled()
-    })
-
-    it('should not sync on page close when nodes are read-only', () => {
-      mockGetNodesReadOnly.mockReturnValue(true)
-
-      // Mock sendBeacon
-      const mockSendBeacon = vi.fn()
-      Object.defineProperty(navigator, 'sendBeacon', {
-        value: mockSendBeacon,
-        writable: true,
-      })
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      act(() => {
-        result.current.syncWorkflowDraftWhenPageClose()
-      })
-
-      expect(mockSendBeacon).not.toHaveBeenCalled()
-    })
-  })
-
-  describe('workflow data not loaded', () => {
-    it('should not sync when workflow data is not loaded', async () => {
-      mockWorkflowStoreGetState.mockReturnValue(
-        createMockWorkflowStoreState({ isWorkflowDataLoaded: false }),
-      )
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSyncWorkflowDraft).not.toHaveBeenCalled()
-    })
-  })
-
-  describe('no appId', () => {
-    it('should not sync when appId is not set', async () => {
-      mockWorkflowStoreGetState.mockReturnValue(
-        createMockWorkflowStoreState({ appId: null }),
-      )
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSyncWorkflowDraft).not.toHaveBeenCalled()
-    })
-  })
-
-  describe('node filtering', () => {
-    it('should filter out temp nodes', async () => {
-      mockGetNodes.mockReturnValue([
-        { id: 'node-1', type: 'start', data: { type: 'start' } },
-        { id: 'node-temp', type: 'custom', data: { type: 'custom', _isTempNode: true } },
-        { id: 'node-2', type: 'llm', data: { type: 'llm' } },
-      ])
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSyncWorkflowDraft).toHaveBeenCalledWith(
-        expect.objectContaining({
-          params: expect.objectContaining({
-            graph: expect.objectContaining({
-              nodes: expect.not.arrayContaining([
-                expect.objectContaining({ id: 'node-temp' }),
-              ]),
-            }),
-          }),
-        }),
-      )
-    })
-
-    it('should remove internal underscore properties from nodes', async () => {
-      mockGetNodes.mockReturnValue([
-        {
-          id: 'node-1',
-          type: 'start',
-          data: {
-            type: 'start',
-            _internalProp: 'should be removed',
-            _anotherInternal: true,
-            publicProp: 'should remain',
-          },
-        },
-      ])
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      const callArgs = mockSyncWorkflowDraft.mock.calls[0][0]
-      const sentNode = callArgs.params.graph.nodes[0]
-
-      expect(sentNode.data).not.toHaveProperty('_internalProp')
-      expect(sentNode.data).not.toHaveProperty('_anotherInternal')
-      expect(sentNode.data).toHaveProperty('publicProp', 'should remain')
-    })
-  })
-
-  describe('edge filtering', () => {
-    it('should filter out temp edges', async () => {
-      mockStoreState.edges = [
-        { id: 'edge-1', source: 'node-1', target: 'node-2', data: {} },
-        { id: 'edge-temp', source: 'node-1', target: 'node-3', data: { _isTemp: true } },
-      ]
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      const callArgs = mockSyncWorkflowDraft.mock.calls[0][0]
-      const sentEdges = callArgs.params.graph.edges
-
-      expect(sentEdges).toHaveLength(1)
-      expect(sentEdges[0].id).toBe('edge-1')
-    })
-
-    it('should remove internal underscore properties from edges', async () => {
-      mockStoreState.edges = [
-        {
-          id: 'edge-1',
-          source: 'node-1',
-          target: 'node-2',
-          data: {
-            _internalEdgeProp: 'should be removed',
-            publicEdgeProp: 'should remain',
-          },
-        },
-      ]
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      const callArgs = mockSyncWorkflowDraft.mock.calls[0][0]
-      const sentEdge = callArgs.params.graph.edges[0]
-
-      expect(sentEdge.data).not.toHaveProperty('_internalEdgeProp')
-      expect(sentEdge.data).toHaveProperty('publicEdgeProp', 'should remain')
-    })
-  })
-
-  describe('viewport handling', () => {
-    it('should send current viewport from transform', async () => {
-      mockStoreState.transform = [100, 200, 1.5]
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      expect(mockSyncWorkflowDraft).toHaveBeenCalledWith(
-        expect.objectContaining({
-          params: expect.objectContaining({
-            graph: expect.objectContaining({
-              viewport: { x: 100, y: 200, zoom: 1.5 },
-            }),
-          }),
-        }),
-      )
-    })
-  })
-
-  describe('multi-tab concurrent editing scenario (END-TO-END TEST)', () => {
-    /**
-     * Simulates the complete multi-tab scenario to verify the fix works correctly.
-     *
-     * Scenario:
-     * 1. Tab A and Tab B both have the workflow open with hash 'hash-v1'
-     * 2. Tab A saves successfully, server returns 'hash-v2'
-     * 3. Tab B tries to save with 'hash-v1', gets 'draft_workflow_not_sync' error
-     * 4. Tab B should only update hash to 'hash-v2', not overwrite canvas
-     * 5. Tab B can now retry save with correct hash
-     */
-    it('should preserve canvas data during hash conflict resolution', async () => {
-      // Initial state: both tabs have hash-v1
-      mockWorkflowStoreGetState.mockReturnValue(
-        createMockWorkflowStoreState({ syncWorkflowDraftHash: 'hash-v1' }),
-      )
-
-      // Tab B tries to save with old hash, server returns error
-      const syncError = createMockErrorResponse('draft_workflow_not_sync')
-      mockSyncWorkflowDraft.mockRejectedValue(syncError)
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      // Tab B attempts to sync
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      // Verify the sync was attempted with old hash
-      expect(mockSyncWorkflowDraft).toHaveBeenCalledWith(
-        expect.objectContaining({
-          params: expect.objectContaining({
-            hash: 'hash-v1',
-          }),
-        }),
-      )
-
-      // Verify handleRefreshWorkflowDraft was called with true (not overwrite canvas)
-      await waitFor(() => {
-        expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledWith(true)
-      })
-
-      // The key assertion: only one argument (true) was passed
-      expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledTimes(1)
-      expect(mockHandleRefreshWorkflowDraft.mock.calls[0]).toEqual([true])
-    })
-
-    it('should handle multiple consecutive sync failures gracefully', async () => {
-      // Create fresh error for each call to avoid bodyUsed issue
-      mockSyncWorkflowDraft
-        .mockRejectedValueOnce(createMockErrorResponse('draft_workflow_not_sync'))
-        .mockRejectedValueOnce(createMockErrorResponse('draft_workflow_not_sync'))
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      // First sync attempt
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      // Wait for first refresh call
-      await waitFor(() => {
-        expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledTimes(1)
-      })
-
-      // Second sync attempt
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft()
-      })
-
-      // Both should call handleRefreshWorkflowDraft with true
-      await waitFor(() => {
-        expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledTimes(2)
-      })
-
-      mockHandleRefreshWorkflowDraft.mock.calls.forEach((call) => {
-        expect(call).toEqual([true])
-      })
-    })
-  })
-
-  describe('callbacks behavior', () => {
-    it('should not call onSuccess when sync fails', async () => {
-      const syncError = createMockErrorResponse('draft_workflow_not_sync')
-      mockSyncWorkflowDraft.mockRejectedValue(syncError)
-      const onSuccess = vi.fn()
-      const onError = vi.fn()
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onSuccess, onError })
-      })
-
-      expect(onSuccess).not.toHaveBeenCalled()
-      expect(onError).toHaveBeenCalled()
-    })
-
-    it('should always call onSettled regardless of success or failure', async () => {
-      const onSettled = vi.fn()
-
-      const { result } = renderHook(() => useNodesSyncDraft())
-
-      // Test success case
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onSettled })
-      })
-      expect(onSettled).toHaveBeenCalledTimes(1)
-
-      // Reset
-      onSettled.mockClear()
-
-      // Test failure case
-      const syncError = createMockErrorResponse('draft_workflow_not_sync')
-      mockSyncWorkflowDraft.mockRejectedValue(syncError)
-
-      await act(async () => {
-        await result.current.doSyncWorkflowDraft(false, { onSettled })
-      })
-      expect(onSettled).toHaveBeenCalledTimes(1)
-    })
-  })
-})

web/app/components/workflow-app/hooks/use-nodes-sync-draft.ts

@@ -115,7 +115,7 @@ export const useNodesSyncDraft = () => {
         if (error && error.json && !error.bodyUsed) {
           error.json().then((err: any) => {
             if (err.code === 'draft_workflow_not_sync' && !notRefreshWhenSyncError)
-              handleRefreshWorkflowDraft(true)
+              handleRefreshWorkflowDraft()
           })
         }
         callback?.onError?.()

web/app/components/workflow-app/hooks/use-workflow-refresh-draft.spec.ts

@@ -1,556 +0,0 @@
-/**
- * Test Suite for useWorkflowRefreshDraft Hook
- *
- * PURPOSE:
- * This hook is responsible for refreshing workflow draft data from the server.
- * The key fix being tested is the `notUpdateCanvas` parameter behavior.
- *
- * MULTI-TAB PROBLEM SCENARIO:
- * 1. User opens the same workflow in Tab A and Tab B (both have hash: v1)
- * 2. Tab A saves successfully, server returns new hash: v2
- * 3. Tab B tries to save with old hash: v1, server returns 400 error (draft_workflow_not_sync)
- * 4. BEFORE FIX: handleRefreshWorkflowDraft() was called without args, which fetched
- *    draft AND overwrote canvas - user lost unsaved changes in Tab B
- * 5. AFTER FIX: handleRefreshWorkflowDraft(true) is called, which fetches draft but
- *    only updates hash, preserving user's canvas changes
- *
- * TESTING STRATEGY:
- * We don't simulate actual tab switching UI behavior. Instead, we test the hook's
- * response to specific inputs:
- * - When notUpdateCanvas=true: should NOT call handleUpdateWorkflowCanvas
- * - When notUpdateCanvas=false/undefined: should call handleUpdateWorkflowCanvas
- *
- * This is behavior-driven testing - we verify "what the code does when given specific
- * inputs" rather than simulating complete user interaction flows.
- */
-
-import { act, renderHook, waitFor } from '@testing-library/react'
-import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { useWorkflowRefreshDraft } from './use-workflow-refresh-draft'
-
-// Mock the workflow service
-const mockFetchWorkflowDraft = vi.fn()
-vi.mock('@/service/workflow', () => ({
-  fetchWorkflowDraft: (...args: unknown[]) => mockFetchWorkflowDraft(...args),
-}))
-
-// Mock the workflow update hook
-const mockHandleUpdateWorkflowCanvas = vi.fn()
-vi.mock('@/app/components/workflow/hooks', () => ({
-  useWorkflowUpdate: () => ({
-    handleUpdateWorkflowCanvas: mockHandleUpdateWorkflowCanvas,
-  }),
-}))
-
-// Mock store state
-const mockSetSyncWorkflowDraftHash = vi.fn()
-const mockSetIsSyncingWorkflowDraft = vi.fn()
-const mockSetEnvironmentVariables = vi.fn()
-const mockSetEnvSecrets = vi.fn()
-const mockSetConversationVariables = vi.fn()
-const mockSetIsWorkflowDataLoaded = vi.fn()
-const mockCancelDebouncedSync = vi.fn()
-
-const createMockStoreState = (overrides = {}) => ({
-  appId: 'test-app-id',
-  setSyncWorkflowDraftHash: mockSetSyncWorkflowDraftHash,
-  setIsSyncingWorkflowDraft: mockSetIsSyncingWorkflowDraft,
-  setEnvironmentVariables: mockSetEnvironmentVariables,
-  setEnvSecrets: mockSetEnvSecrets,
-  setConversationVariables: mockSetConversationVariables,
-  setIsWorkflowDataLoaded: mockSetIsWorkflowDataLoaded,
-  isWorkflowDataLoaded: true,
-  debouncedSyncWorkflowDraft: {
-    cancel: mockCancelDebouncedSync,
-  },
-  ...overrides,
-})
-
-const mockWorkflowStoreGetState = vi.fn()
-vi.mock('@/app/components/workflow/store', () => ({
-  useWorkflowStore: () => ({
-    getState: mockWorkflowStoreGetState,
-  }),
-}))
-
-// Default mock response from fetchWorkflowDraft
-const createMockDraftResponse = (overrides = {}) => ({
-  hash: 'new-hash-12345',
-  graph: {
-    nodes: [{ id: 'node-1', type: 'start', data: {} }],
-    edges: [{ id: 'edge-1', source: 'node-1', target: 'node-2' }],
-    viewport: { x: 100, y: 200, zoom: 1.5 },
-  },
-  environment_variables: [
-    { id: 'env-1', name: 'API_KEY', value: 'secret-key', value_type: 'secret' },
-    { id: 'env-2', name: 'BASE_URL', value: 'https://api.example.com', value_type: 'string' },
-  ],
-  conversation_variables: [
-    { id: 'conv-1', name: 'user_input', value: 'test' },
-  ],
-  ...overrides,
-})
-
-describe('useWorkflowRefreshDraft', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-    mockWorkflowStoreGetState.mockReturnValue(createMockStoreState())
-    mockFetchWorkflowDraft.mockResolvedValue(createMockDraftResponse())
-  })
-
-  afterEach(() => {
-    vi.resetAllMocks()
-  })
-
-  describe('handleRefreshWorkflowDraft function', () => {
-    it('should return handleRefreshWorkflowDraft function', () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      expect(result.current.handleRefreshWorkflowDraft).toBeDefined()
-      expect(typeof result.current.handleRefreshWorkflowDraft).toBe('function')
-    })
-  })
-
-  describe('notUpdateCanvas parameter behavior (THE KEY FIX)', () => {
-    it('should NOT call handleUpdateWorkflowCanvas when notUpdateCanvas is true', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockFetchWorkflowDraft).toHaveBeenCalledWith('/apps/test-app-id/workflows/draft')
-      })
-
-      await waitFor(() => {
-        expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345')
-      })
-
-      // THE KEY ASSERTION: Canvas should NOT be updated when notUpdateCanvas is true
-      expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled()
-    })
-
-    it('should call handleUpdateWorkflowCanvas when notUpdateCanvas is false', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(false)
-      })
-
-      await waitFor(() => {
-        expect(mockFetchWorkflowDraft).toHaveBeenCalledWith('/apps/test-app-id/workflows/draft')
-      })
-
-      await waitFor(() => {
-        // Canvas SHOULD be updated when notUpdateCanvas is false
-        expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledWith({
-          nodes: [{ id: 'node-1', type: 'start', data: {} }],
-          edges: [{ id: 'edge-1', source: 'node-1', target: 'node-2' }],
-          viewport: { x: 100, y: 200, zoom: 1.5 },
-        })
-      })
-
-      await waitFor(() => {
-        expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345')
-      })
-    })
-
-    it('should call handleUpdateWorkflowCanvas when notUpdateCanvas is undefined (default)', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      await waitFor(() => {
-        expect(mockFetchWorkflowDraft).toHaveBeenCalled()
-      })
-
-      await waitFor(() => {
-        // Canvas SHOULD be updated when notUpdateCanvas is undefined
-        expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalled()
-      })
-    })
-
-    it('should still update hash even when notUpdateCanvas is true', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345')
-      })
-
-      // Verify canvas was NOT updated
-      expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled()
-    })
-
-    it('should still update environment variables when notUpdateCanvas is true', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetEnvironmentVariables).toHaveBeenCalledWith([
-          { id: 'env-1', name: 'API_KEY', value: '[__HIDDEN__]', value_type: 'secret' },
-          { id: 'env-2', name: 'BASE_URL', value: 'https://api.example.com', value_type: 'string' },
-        ])
-      })
-
-      expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled()
-    })
-
-    it('should still update env secrets when notUpdateCanvas is true', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetEnvSecrets).toHaveBeenCalledWith({
-          'env-1': 'secret-key',
-        })
-      })
-
-      expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled()
-    })
-
-    it('should still update conversation variables when notUpdateCanvas is true', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetConversationVariables).toHaveBeenCalledWith([
-          { id: 'conv-1', name: 'user_input', value: 'test' },
-        ])
-      })
-
-      expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled()
-    })
-  })
-
-  describe('syncing state management', () => {
-    it('should set isSyncingWorkflowDraft to true before fetch', () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      expect(mockSetIsSyncingWorkflowDraft).toHaveBeenCalledWith(true)
-    })
-
-    it('should set isSyncingWorkflowDraft to false after fetch completes', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      await waitFor(() => {
-        expect(mockSetIsSyncingWorkflowDraft).toHaveBeenCalledWith(false)
-      })
-    })
-
-    it('should set isSyncingWorkflowDraft to false even when fetch fails', async () => {
-      mockFetchWorkflowDraft.mockRejectedValue(new Error('Network error'))
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      await waitFor(() => {
-        expect(mockSetIsSyncingWorkflowDraft).toHaveBeenCalledWith(false)
-      })
-    })
-  })
-
-  describe('isWorkflowDataLoaded flag management', () => {
-    it('should set isWorkflowDataLoaded to false before fetch when it was true', () => {
-      mockWorkflowStoreGetState.mockReturnValue(
-        createMockStoreState({ isWorkflowDataLoaded: true }),
-      )
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      expect(mockSetIsWorkflowDataLoaded).toHaveBeenCalledWith(false)
-    })
-
-    it('should set isWorkflowDataLoaded to true after fetch succeeds', async () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      await waitFor(() => {
-        expect(mockSetIsWorkflowDataLoaded).toHaveBeenCalledWith(true)
-      })
-    })
-
-    it('should restore isWorkflowDataLoaded when fetch fails and it was previously loaded', async () => {
-      mockWorkflowStoreGetState.mockReturnValue(
-        createMockStoreState({ isWorkflowDataLoaded: true }),
-      )
-      mockFetchWorkflowDraft.mockRejectedValue(new Error('Network error'))
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      await waitFor(() => {
-        // Should restore to true because wasLoaded was true
-        expect(mockSetIsWorkflowDataLoaded).toHaveBeenLastCalledWith(true)
-      })
-    })
-  })
-
-  describe('debounced sync cancellation', () => {
-    it('should cancel debounced sync before fetching draft', () => {
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft()
-      })
-
-      expect(mockCancelDebouncedSync).toHaveBeenCalled()
-    })
-
-    it('should handle case when debouncedSyncWorkflowDraft has no cancel method', () => {
-      mockWorkflowStoreGetState.mockReturnValue(
-        createMockStoreState({ debouncedSyncWorkflowDraft: {} }),
-      )
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      // Should not throw
-      expect(() => {
-        act(() => {
-          result.current.handleRefreshWorkflowDraft()
-        })
-      }).not.toThrow()
-    })
-  })
-
-  describe('edge cases', () => {
-    it('should handle empty graph in response', async () => {
-      mockFetchWorkflowDraft.mockResolvedValue({
-        hash: 'hash-empty',
-        graph: null,
-        environment_variables: [],
-        conversation_variables: [],
-      })
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(false)
-      })
-
-      await waitFor(() => {
-        expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledWith({
-          nodes: [],
-          edges: [],
-          viewport: { x: 0, y: 0, zoom: 1 },
-        })
-      })
-    })
-
-    it('should handle missing viewport in response', async () => {
-      mockFetchWorkflowDraft.mockResolvedValue({
-        hash: 'hash-no-viewport',
-        graph: {
-          nodes: [{ id: 'node-1' }],
-          edges: [],
-          viewport: null,
-        },
-        environment_variables: [],
-        conversation_variables: [],
-      })
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(false)
-      })
-
-      await waitFor(() => {
-        expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledWith({
-          nodes: [{ id: 'node-1' }],
-          edges: [],
-          viewport: { x: 0, y: 0, zoom: 1 },
-        })
-      })
-    })
-
-    it('should handle missing environment_variables in response', async () => {
-      mockFetchWorkflowDraft.mockResolvedValue({
-        hash: 'hash-no-env',
-        graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } },
-        environment_variables: undefined,
-        conversation_variables: [],
-      })
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetEnvironmentVariables).toHaveBeenCalledWith([])
-        expect(mockSetEnvSecrets).toHaveBeenCalledWith({})
-      })
-    })
-
-    it('should handle missing conversation_variables in response', async () => {
-      mockFetchWorkflowDraft.mockResolvedValue({
-        hash: 'hash-no-conv',
-        graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } },
-        environment_variables: [],
-        conversation_variables: undefined,
-      })
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetConversationVariables).toHaveBeenCalledWith([])
-      })
-    })
-
-    it('should filter only secret type for envSecrets', async () => {
-      mockFetchWorkflowDraft.mockResolvedValue({
-        hash: 'hash-mixed-env',
-        graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } },
-        environment_variables: [
-          { id: 'env-1', name: 'SECRET_KEY', value: 'secret-value', value_type: 'secret' },
-          { id: 'env-2', name: 'PUBLIC_URL', value: 'https://example.com', value_type: 'string' },
-          { id: 'env-3', name: 'ANOTHER_SECRET', value: 'another-secret', value_type: 'secret' },
-        ],
-        conversation_variables: [],
-      })
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetEnvSecrets).toHaveBeenCalledWith({
-          'env-1': 'secret-value',
-          'env-3': 'another-secret',
-        })
-      })
-    })
-
-    it('should hide secret values in environment variables', async () => {
-      mockFetchWorkflowDraft.mockResolvedValue({
-        hash: 'hash-secrets',
-        graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } },
-        environment_variables: [
-          { id: 'env-1', name: 'SECRET_KEY', value: 'super-secret', value_type: 'secret' },
-          { id: 'env-2', name: 'PUBLIC_URL', value: 'https://example.com', value_type: 'string' },
-        ],
-        conversation_variables: [],
-      })
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockSetEnvironmentVariables).toHaveBeenCalledWith([
-          { id: 'env-1', name: 'SECRET_KEY', value: '[__HIDDEN__]', value_type: 'secret' },
-          { id: 'env-2', name: 'PUBLIC_URL', value: 'https://example.com', value_type: 'string' },
-        ])
-      })
-    })
-  })
-
-  describe('multi-tab scenario simulation (THE BUG FIX VERIFICATION)', () => {
-    /**
-     * This test verifies the fix for the multi-tab scenario:
-     * 1. User opens workflow in Tab A and Tab B
-     * 2. Tab A saves draft successfully
-     * 3. Tab B tries to save but gets 'draft_workflow_not_sync' error (hash mismatch)
-     * 4. BEFORE FIX: Tab B would fetch draft and overwrite canvas with old data
-     * 5. AFTER FIX: Tab B only updates hash, preserving user's canvas changes
-     */
-    it('should only update hash when called with notUpdateCanvas=true (simulating sync error recovery)', async () => {
-      const mockResponse = createMockDraftResponse()
-      mockFetchWorkflowDraft.mockResolvedValue(mockResponse)
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      // Simulate the sync error recovery scenario where notUpdateCanvas is true
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(true)
-      })
-
-      await waitFor(() => {
-        expect(mockFetchWorkflowDraft).toHaveBeenCalled()
-      })
-
-      await waitFor(() => {
-        // Hash should be updated for next sync attempt
-        expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345')
-      })
-
-      // Canvas should NOT be updated - user's changes are preserved
-      expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled()
-
-      // Other states should still be updated
-      expect(mockSetEnvironmentVariables).toHaveBeenCalled()
-      expect(mockSetConversationVariables).toHaveBeenCalled()
-    })
-
-    it('should update canvas when called with notUpdateCanvas=false (normal refresh)', async () => {
-      const mockResponse = createMockDraftResponse()
-      mockFetchWorkflowDraft.mockResolvedValue(mockResponse)
-
-      const { result } = renderHook(() => useWorkflowRefreshDraft())
-
-      // Simulate normal refresh scenario
-      act(() => {
-        result.current.handleRefreshWorkflowDraft(false)
-      })
-
-      await waitFor(() => {
-        expect(mockFetchWorkflowDraft).toHaveBeenCalled()
-      })
-
-      await waitFor(() => {
-        expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345')
-      })
-
-      // Canvas SHOULD be updated in normal refresh
-      await waitFor(() => {
-        expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalled()
-      })
-    })
-  })
-})

web/app/components/workflow-app/hooks/use-workflow-refresh-draft.ts

@@ -8,7 +8,7 @@ export const useWorkflowRefreshDraft = () => {
   const workflowStore = useWorkflowStore()
   const { handleUpdateWorkflowCanvas } = useWorkflowUpdate()
 
-  const handleRefreshWorkflowDraft = useCallback((notUpdateCanvas?: boolean) => {
+  const handleRefreshWorkflowDraft = useCallback(() => {
     const {
       appId,
       setSyncWorkflowDraftHash,
@@ -31,14 +31,12 @@ export const useWorkflowRefreshDraft = () => {
     fetchWorkflowDraft(`/apps/${appId}/workflows/draft`)
       .then((response) => {
         // Ensure we have a valid workflow structure with viewport
-        if (!notUpdateCanvas) {
-          const workflowData: WorkflowDataUpdater = {
-            nodes: response.graph?.nodes || [],
-            edges: response.graph?.edges || [],
-            viewport: response.graph?.viewport || { x: 0, y: 0, zoom: 1 },
-          }
-          handleUpdateWorkflowCanvas(workflowData)
+        const workflowData: WorkflowDataUpdater = {
+          nodes: response.graph?.nodes || [],
+          edges: response.graph?.edges || [],
+          viewport: response.graph?.viewport || { x: 0, y: 0, zoom: 1 },
         }
+        handleUpdateWorkflowCanvas(workflowData)
         setSyncWorkflowDraftHash(response.hash)
         setEnvSecrets((response.environment_variables || []).filter(env => env.value_type === 'secret').reduce((acc, env) => {
           acc[env.id] = env.value