fix: deduplicate workflow and job names (#4606)

* fix: deduplicate workflow and job names on import

Workflow name validation and job name deduplication now happen centrally
inside importWorkflow, fixing AI-generated workflows that couldn't be
saved when they had a name collision with existing workflows or
duplicate job names.

* test: add importWorkflow tests for name deduplication

Covers workflow name validation via channel, job name deduplication,
graceful fallback when validation fails, and no-op for unique names.

* fix: avoid collisions when dedup suffix matches an existing job name

Use a Set to track all used names (original + generated) instead of a
simple counter, so "Transform 2" is skipped when it already exists.

* test: fix mocks to return Promise for async importWorkflow

* docs: add changelog entry for workflow name dedup fix

* fix: handle importWorkflow rejection and reduce test boilerplate

Add .catch() to the importWorkflow promise in WorkflowEditor to prevent
unhandled rejections. Extract makeJob, makeWorkflowState, and getJobNames
test helpers to reduce repetition in importWorkflow tests.

* fix: resolve TypeScript errors for job_id on WorkflowTemplateContext union

Extract selectedContextJobId with an `in` type guard to safely access
job_id from the WorkflowTemplateContext discriminated union, fixing TS2339
errors and an eslint complex-expression warning.

---------

Co-authored-by: Frank Midigo <midigofrank@gmail.com>

Author: elias-ba

CHANGELOG.md

@@ -39,6 +39,9 @@ and this project adheres to
 
 ### Fixed
 
+- AI-generated workflows can now be saved when the workflow name collides with
+  an existing workflow or when jobs have duplicate names
+  [#4607](https://github.com/OpenFn/lightning/issues/4607)
 - Since OTP26, if `SMTP_PROVIDER` is set to `smtp` and `SMTP_TLS` is set to
   `true` or `if_available` this would result in TLS-related failures when trying
   to send emails. This is now fixed for a limited number of use cases (see

assets/js/collaborative-editor/components/AIAssistantPanel.tsx

@@ -164,6 +164,11 @@ export function AIAssistantPanel({
           ? 'Connecting...'
           : undefined;
 
+  const selectedContextJobId =
+    workflowTemplateContext && 'job_id' in workflowTemplateContext
+      ? workflowTemplateContext.job_id
+      : undefined;
+
   // Load session list when viewing sessions
   useEffect(() => {
     if (!isOpen || view !== 'sessions' || !storeSessionType) return;
@@ -177,7 +182,7 @@ export function AIAssistantPanel({
     isOpen,
     view,
     storeSessionType,
-    workflowTemplateContext?.job_id,
+    selectedContextJobId,
     hasSessionContext,
     loadSessionList,
   ]);
@@ -462,7 +467,7 @@ export function AIAssistantPanel({
         disabledMessage={disabledMessage}
         selectedStepId={selectedStepId}
         selectedRunId={selectedRunId}
-        selectedJobId={workflowTemplateContext?.job_id ?? null}
+        selectedJobId={selectedContextJobId ?? null}
       />
 
       {/* About AI Assistant Modal */}

assets/js/collaborative-editor/components/WorkflowEditor.tsx

@@ -393,19 +393,13 @@ export function WorkflowEditor({
    */
   const handleImport = useCallback(
     (workflowState: YAMLWorkflowState) => {
-      // Validate workflow name asynchronously, but proceed with import regardless
-      workflowStore
-        .validateWorkflowName(workflowState)
-        .then(validatedState => {
-          void workflowStore.importWorkflow(validatedState);
+      void workflowStore
+        .importWorkflow(workflowState)
+        .then(() => {
           flowEvents.dispatch('fit-view');
-          return;
         })
         .catch((error: unknown) => {
-          // If validation fails, import with original state
-          // Server will handle any name conflicts on save
-          console.warn('Workflow name validation failed, proceeding:', error);
-          workflowStore.importWorkflow(workflowState);
+          console.error('Failed to import workflow:', error);
         });
     },
     [workflowStore]

assets/js/collaborative-editor/hooks/useAIWorkflowApplications.ts

@@ -123,7 +123,7 @@ export function useAIWorkflowApplications({
   currentUserId: string | undefined;
   aiMode: AIModeResult | null;
   workflowActions: {
-    importWorkflow: (state: YAMLWorkflowState) => void;
+    importWorkflow: (state: YAMLWorkflowState) => Promise<void>;
     startApplyingWorkflow: (messageId: string) => Promise<boolean>;
     doneApplyingWorkflow: (messageId: string) => Promise<void>;
     startApplyingJobCode: (messageId: string) => Promise<boolean>;
@@ -198,7 +198,7 @@ export function useAIWorkflowApplications({
           extractJobCredentials(jobs)
         );
 
-        importWorkflow(workflowStateWithCreds);
+        await importWorkflow(workflowStateWithCreds);
       } catch (error) {
         console.error('[AI Assistant] Failed to apply workflow:', error);
 

assets/js/collaborative-editor/stores/createWorkflowStore.ts

@@ -1540,28 +1540,91 @@ export const createWorkflowStore = () => {
     }
   };
 
+  /**
+   * Deduplicate job names within a workflow state.
+   *
+   * If multiple jobs share the same name, appends " 2", " 3", etc.
+   * to make each name unique. The first occurrence keeps its original name.
+   * Skips suffixes that collide with existing names in the workflow.
+   */
+  const deduplicateJobNames = (
+    workflowState: YAMLWorkflowState
+  ): YAMLWorkflowState => {
+    const allOriginalNames = new Set(workflowState.jobs.map(j => j.name));
+    const usedNames = new Set<string>();
+    let hasDuplicates = false;
+
+    const jobs = workflowState.jobs.map(job => {
+      if (!usedNames.has(job.name)) {
+        usedNames.add(job.name);
+        return job;
+      }
+
+      hasDuplicates = true;
+      let counter = 2;
+      while (
+        usedNames.has(`${job.name} ${counter}`) ||
+        allOriginalNames.has(`${job.name} ${counter}`)
+      ) {
+        counter++;
+      }
+      const newName = `${job.name} ${counter}`;
+      usedNames.add(newName);
+      return { ...job, name: newName };
+    });
+
+    if (hasDuplicates) {
+      logger.info('Deduplicated job names', {
+        original: workflowState.jobs.map(j => j.name),
+        deduplicated: jobs.map(j => j.name),
+      });
+      return { ...workflowState, jobs };
+    }
+
+    return workflowState;
+  };
+
   /**
    * Import workflow from YAML WorkflowState
    *
+   * Validates workflow name uniqueness via the server and deduplicates
+   * job names client-side before writing to Y.Doc. This is the single
+   * central place for name validation on import, regardless of how the
+   * import was triggered (manual import, template, or AI generation).
+   *
    * Uses Pattern 1 (Y.Doc → Observer → Immer):
    * - Single transact() for atomic bulk updates
    * - Observers automatically sync Immer state
    * - No manual notify() calls needed
    *
    * @param workflowState - Parsed YAML workflow state
    */
-  const importWorkflow = (workflowState: YAMLWorkflowState) => {
+  const importWorkflow = async (workflowState: YAMLWorkflowState) => {
     const ydoc = ensureYDoc();
 
+    // Validate workflow name uniqueness via server
+    let validatedState: YAMLWorkflowState;
+    try {
+      validatedState = await validateWorkflowName(workflowState);
+    } catch (error) {
+      // If validation fails, proceed with original name
+      // Server will handle any name conflicts on save
+      logger.warn('Workflow name validation failed, proceeding', error);
+      validatedState = workflowState;
+    }
+
+    // Deduplicate job names client-side
+    validatedState = deduplicateJobNames(validatedState);
+
     try {
       // Use adapter to apply transformations and update Y.Doc
-      YAMLStateToYDoc.applyToYDoc(ydoc, workflowState);
+      YAMLStateToYDoc.applyToYDoc(ydoc, validatedState);
 
       logger.info('Workflow imported successfully', {
-        workflowId: workflowState.id,
-        jobs: workflowState.jobs.length,
-        triggers: workflowState.triggers.length,
-        edges: workflowState.edges.length,
+        workflowId: validatedState.id,
+        jobs: validatedState.jobs.length,
+        triggers: validatedState.triggers.length,
+        edges: validatedState.edges.length,
       });
 
       // Note: Observers will automatically trigger Immer updates and notify React

assets/test/collaborative-editor/components/WorkflowEditor.test.tsx

@@ -238,7 +238,7 @@ vi.mock('../../../js/collaborative-editor/hooks/useWorkflow', () => ({
   }),
   useWorkflowStoreContext: () => ({
     validateWorkflowName: vi.fn(),
-    importWorkflow: vi.fn(),
+    importWorkflow: vi.fn().mockResolvedValue(undefined),
   }),
   useWorkflowActions: () => ({
     saveWorkflow: vi.fn(),

assets/test/collaborative-editor/hooks/useAIWorkflowApplications.autoApply.test.ts

@@ -68,7 +68,7 @@ vi.mock('../../../js/yaml/util', () => ({
 
 describe('useAIWorkflowApplications - Auto-Application', () => {
   // Mock functions
-  const mockImportWorkflow = vi.fn();
+  const mockImportWorkflow = vi.fn(() => Promise.resolve());
   const mockStartApplyingWorkflow = vi.fn(() => Promise.resolve(true));
   const mockDoneApplyingWorkflow = vi.fn(() => Promise.resolve());
   const mockStartApplyingJobCode = vi.fn(() => Promise.resolve(true));

assets/test/collaborative-editor/hooks/useAIWorkflowApplications.workflow.test.ts

@@ -83,7 +83,7 @@ vi.mock('../../../js/collaborative-editor/lib/notifications', () => ({
 
 describe('useAIWorkflowApplications - handleApplyWorkflow', () => {
   // Mock functions
-  const mockImportWorkflow = vi.fn();
+  const mockImportWorkflow = vi.fn(() => Promise.resolve());
   const mockStartApplyingWorkflow = vi.fn(() => Promise.resolve(true));
   const mockDoneApplyingWorkflow = vi.fn(() => Promise.resolve());
   const mockStartApplyingJobCode = vi.fn(() => Promise.resolve(true));

assets/test/collaborative-editor/stores/createWorkflowStore.test.ts

@@ -20,6 +20,7 @@ import { ChannelRequestError } from '../../../js/collaborative-editor/lib/errors
 import { createWorkflowStore } from '../../../js/collaborative-editor/stores/createWorkflowStore';
 import type { WorkflowStoreInstance } from '../../../js/collaborative-editor/stores/createWorkflowStore';
 import type { Session } from '../../../js/collaborative-editor/types/session';
+import type { WorkflowState as YAMLWorkflowState } from '../../../js/yaml/types';
 import {
   createMockChannelPushOk,
   createMockChannelPushError,
@@ -1280,3 +1281,133 @@ describe('WorkflowStore - AI Workflow Apply Coordination', () => {
     ).resolves.not.toThrow();
   });
 });
+
+describe('WorkflowStore - importWorkflow', () => {
+  let store: WorkflowStoreInstance;
+  let ydoc: Session.WorkflowDoc;
+  let mockChannel: MockPhoenixChannel;
+  let mockProvider: PhoenixChannelProvider & { channel: MockPhoenixChannel };
+
+  const makeJob = (id: string, name: string) => ({
+    id,
+    name,
+    adaptor: '@openfn/language-http@latest',
+    body: 'fn(s => s)',
+    project_credential_id: null,
+    keychain_credential_id: null,
+  });
+
+  const makeWorkflowState = (
+    overrides: Partial<YAMLWorkflowState> = {}
+  ): YAMLWorkflowState => ({
+    id: 'wf-1',
+    name: 'My Workflow',
+    jobs: [],
+    triggers: [],
+    edges: [],
+    positions: null,
+    ...overrides,
+  });
+
+  const getJobNames = () => {
+    const jobsArray = ydoc.getArray('jobs');
+    return Array.from({ length: jobsArray.length }, (_, i) =>
+      (jobsArray.get(i) as Y.Map<unknown>).get('name')
+    );
+  };
+
+  beforeEach(() => {
+    store = createWorkflowStore();
+    ydoc = new Y.Doc() as Session.WorkflowDoc;
+
+    ydoc.getArray('jobs');
+    ydoc.getMap('workflow');
+    ydoc.getArray('triggers');
+    ydoc.getArray('edges');
+    ydoc.getMap('positions');
+
+    mockChannel = createMockPhoenixChannel('workflow:test');
+
+    mockProvider = {
+      channel: mockChannel,
+      synced: true,
+      awareness: null,
+      doc: ydoc,
+    } as unknown as PhoenixChannelProvider & { channel: MockPhoenixChannel };
+
+    store.connect(ydoc, mockProvider);
+  });
+
+  test('deduplicates job names when multiple jobs share the same name', async () => {
+    mockChannel.push = createMockChannelPushOk({
+      workflow: { name: 'My Workflow' },
+    }) as typeof mockChannel.push;
+
+    await store.importWorkflow(
+      makeWorkflowState({
+        jobs: [
+          makeJob('j1', 'Transform'),
+          makeJob('j2', 'Transform'),
+          makeJob('j3', 'Transform'),
+        ],
+      })
+    );
+
+    expect(getJobNames()).toEqual(['Transform', 'Transform 2', 'Transform 3']);
+  });
+
+  test('skips suffixes that collide with existing job names', async () => {
+    mockChannel.push = createMockChannelPushOk({
+      workflow: { name: 'My Workflow' },
+    }) as typeof mockChannel.push;
+
+    await store.importWorkflow(
+      makeWorkflowState({
+        jobs: [
+          makeJob('j1', 'Transform'),
+          makeJob('j2', 'Transform'),
+          makeJob('j3', 'Transform 2'),
+        ],
+      })
+    );
+
+    // "Transform 2" is taken, so the duplicate skips to "Transform 3"
+    expect(getJobNames()).toEqual(['Transform', 'Transform 3', 'Transform 2']);
+  });
+
+  test('leaves unique job names unchanged', async () => {
+    mockChannel.push = createMockChannelPushOk({
+      workflow: { name: 'My Workflow' },
+    }) as typeof mockChannel.push;
+
+    await store.importWorkflow(
+      makeWorkflowState({
+        jobs: [makeJob('j1', 'Fetch Data'), makeJob('j2', 'Transform')],
+      })
+    );
+
+    expect(getJobNames()).toEqual(['Fetch Data', 'Transform']);
+  });
+
+  test('validates workflow name via channel', async () => {
+    // Server returns deduplicated name
+    mockChannel.push = createMockChannelPushOk({
+      workflow: { name: 'My Workflow 1' },
+    }) as typeof mockChannel.push;
+
+    await store.importWorkflow(makeWorkflowState());
+
+    expect(ydoc.getMap('workflow').get('name')).toBe('My Workflow 1');
+  });
+
+  test('proceeds with original name when validation fails', async () => {
+    mockChannel.push = createMockChannelPushError(
+      'server_error',
+      {}
+    ) as typeof mockChannel.push;
+
+    await store.importWorkflow(makeWorkflowState());
+
+    expect(ydoc.getMap('workflow').get('name')).toBe('My Workflow');
+  });
+});