feat(notebook-migration, frontend): upload notebooks under a per-work…

frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.spec.ts

@@ -20,7 +20,6 @@
 import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing";
 import { JupyterNotebookPanelComponent } from "./jupyter-notebook-panel.component";
 import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service";
-import { NotebookMigrationService } from "../../service/notebook-migration/notebook-migration.service";
 import { Subject } from "rxjs";
 import { ElementRef } from "@angular/core";
 import { By, DomSanitizer } from "@angular/platform-browser";
@@ -30,7 +29,6 @@ describe("JupyterNotebookPanelComponent", () => {
   let fixture: ComponentFixture<JupyterNotebookPanelComponent>;
 
   let mockJupyterPanelService: any;
-  let mockNotebookMigrationService: any;
   let bypassSpy: ReturnType<typeof vi.spyOn>;
 
   beforeEach(async () => {
@@ -39,18 +37,12 @@ describe("JupyterNotebookPanelComponent", () => {
       setIframeRef: vi.fn(),
       deleteJupyterNotebook: vi.fn(),
       minimizeJupyterNotebookPanel: vi.fn(),
-    };
-
-    mockNotebookMigrationService = {
-      getJupyterIframeURL: vi.fn().mockResolvedValue("http://localhost:8888"),
+      getJupyterIframeURLForWorkflow: vi.fn().mockResolvedValue("http://localhost:8888"),
     };
 
     await TestBed.configureTestingModule({
       imports: [JupyterNotebookPanelComponent],
-      providers: [
-        { provide: JupyterPanelService, useValue: mockJupyterPanelService },
-        { provide: NotebookMigrationService, useValue: mockNotebookMigrationService },
-      ],
+      providers: [{ provide: JupyterPanelService, useValue: mockJupyterPanelService }],
     }).compileComponents();
   });
 
@@ -98,7 +90,7 @@ describe("JupyterNotebookPanelComponent", () => {
     await fixture.whenStable();
     fixture.detectChanges();
 
-    expect(mockNotebookMigrationService.getJupyterIframeURL).toHaveBeenCalled();
+    expect(mockJupyterPanelService.getJupyterIframeURLForWorkflow).toHaveBeenCalled();
     expect(bypassSpy).toHaveBeenCalledWith("http://localhost:8888");
     expect(component.jupyterUrl).toBe(bypassSpy.mock.results[0].value);
   });
@@ -144,20 +136,20 @@ describe("JupyterNotebookPanelComponent", () => {
   it("should not update jupyterUrl when the iframe URL fetch rejects", async () => {
     vi.spyOn(component, "checkIframeRef").mockImplementation(() => {});
     vi.spyOn(console, "error").mockImplementation(() => {});
-    mockNotebookMigrationService.getJupyterIframeURL.mockRejectedValueOnce(new Error("network error"));
+    mockJupyterPanelService.getJupyterIframeURLForWorkflow.mockRejectedValueOnce(new Error("network error"));
 
     mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true);
 
     await fixture.whenStable();
 
-    expect(mockNotebookMigrationService.getJupyterIframeURL).toHaveBeenCalled();
+    expect(mockJupyterPanelService.getJupyterIframeURLForWorkflow).toHaveBeenCalled();
     expect(component.jupyterUrl).toBeNull();
   });
 
   it("should keep handling visibility emissions after a failed fetch", async () => {
     vi.spyOn(component, "checkIframeRef").mockImplementation(() => {});
     vi.spyOn(console, "error").mockImplementation(() => {});
-    mockNotebookMigrationService.getJupyterIframeURL
+    mockJupyterPanelService.getJupyterIframeURLForWorkflow
       .mockRejectedValueOnce(new Error("network error"))
       .mockResolvedValueOnce("http://localhost:9999");
 
@@ -168,7 +160,7 @@ describe("JupyterNotebookPanelComponent", () => {
     await fixture.whenStable();
     fixture.detectChanges();
 
-    expect(mockNotebookMigrationService.getJupyterIframeURL).toHaveBeenCalledTimes(2);
+    expect(mockJupyterPanelService.getJupyterIframeURLForWorkflow).toHaveBeenCalledTimes(2);
     expect(bypassSpy).toHaveBeenCalledWith("http://localhost:9999");
     expect(component.jupyterUrl).toBe(bypassSpy.mock.results[0].value);
   });

frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.ts

@@ -22,7 +22,6 @@ import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.s
 import { from, of, Subject } from "rxjs";
 import { catchError, switchMap, takeUntil } from "rxjs/operators";
 import { DomSanitizer, SafeResourceUrl } from "@angular/platform-browser";
-import { NotebookMigrationService } from "../../service/notebook-migration/notebook-migration.service";
 import { CommonModule } from "@angular/common";
 import { DragDropModule } from "@angular/cdk/drag-drop";
 import { NzButtonModule } from "ng-zorro-antd/button";
@@ -45,8 +44,7 @@ export class JupyterNotebookPanelComponent implements OnInit, AfterViewInit, OnD
 
   constructor(
     private jupyterPanelService: JupyterPanelService,
-    private sanitizer: DomSanitizer,
-    private notebookMigrationService: NotebookMigrationService
+    private sanitizer: DomSanitizer
   ) {}
 
   ngOnInit(): void {
@@ -59,7 +57,7 @@ export class JupyterNotebookPanelComponent implements OnInit, AfterViewInit, OnD
             return of(null);
           }
 
-          return from(this.notebookMigrationService.getJupyterIframeURL()).pipe(
+          return from(this.jupyterPanelService.getJupyterIframeURLForWorkflow()).pipe(
             catchError(() => {
               console.error("Failed to fetch Jupyter iframe URL.");
               return of(null);

frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts

@@ -266,7 +266,8 @@ describe("JupyterPanelService", () => {
     // The mapping is stored before the notebook is handed to Jupyter, ...
     expect(mockNotebook.setMapping).toHaveBeenCalledWith("mapping_wid_1", mapping);
     // ... and the 0 came from Jupyter's own answer, not from a thrown error.
-    expect(mockNotebook.sendNotebookToJupyter).toHaveBeenCalledWith(notebook);
+    // Upload uses the wid-derived filename.
+    expect(mockNotebook.sendNotebookToJupyter).toHaveBeenCalledWith(notebook, "notebook_1.ipynb");
     expect(consoleError).not.toHaveBeenCalled();
   });
 
@@ -284,6 +285,33 @@ describe("JupyterPanelService", () => {
     expect(mockNotebook.sendNotebookToJupyter).not.toHaveBeenCalled();
   });
 
+  it("uploads under the fetched workflow's filename even if the current workflow changed", async () => {
+    // Stale-fetch guard: a fetch for wid 2 that resolves after the user switched to wid 1
+    // must still upload as notebook_2.ipynb, not overwrite wid 1's file.
+    mockNotebook.sendNotebookToJupyter = vi.fn().mockResolvedValue(1);
+    mockWorkflow.getWorkflow.mockReturnValue({ wid: 1 });
+    const mapping = { cell_to_operator: {}, operator_to_cell: {} };
+    const notebook = { cells: [] };
+
+    const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(2, 1));
+    httpMock
+      .expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"))
+      .flush({ exists: true, mapping, notebook });
+
+    expect(await resultPromise).toBe(1);
+    expect(mockNotebook.sendNotebookToJupyter).toHaveBeenCalledWith(notebook, "notebook_2.ipynb");
+  });
+
+  // Iframe URL must use the same wid-derived filename as the upload.
+  it("getJupyterIframeURLForWorkflow requests the current workflow's per-workflow filename", async () => {
+    mockNotebook.getJupyterIframeURL = vi.fn().mockResolvedValue("http://iframe");
+
+    const url = await service.getJupyterIframeURLForWorkflow();
+
+    expect(url).toBe("http://iframe");
+    expect(mockNotebook.getJupyterIframeURL).toHaveBeenCalledWith("notebook_1.ipynb");
+  });
+
   // jupyterNotebookExists$ starts false and flips true once init()'s fetch finds
   // a notebook for the workflow; the toolbar's expand button binds to this.
   it("sets jupyterNotebookExists$ true after a workflow's notebook is fetched", async () => {
@@ -705,6 +733,13 @@ describe("JupyterPanelService", () => {
       expect(mockNotification.warning).not.toHaveBeenCalled();
     });
 
+    it("getJupyterIframeURLForWorkflow resolves null without calling the migration service", async () => {
+      mockNotebook.getJupyterIframeURL = vi.fn();
+      const url = await service.getJupyterIframeURLForWorkflow();
+      expect(url).toBeNull();
+      expect(mockNotebook.getJupyterIframeURL).not.toHaveBeenCalled();
+    });
+
     it("onWorkflowComponentClick does not postMessage to the iframe", async () => {
       const mockIframe = {
         contentWindow: { postMessage: vi.fn() },

frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts

@@ -25,7 +25,11 @@ import { HttpClient, HttpHeaders } from "@angular/common/http";
 import { NotificationService } from "src/app/common/service/notification/notification.service";
 import { distinctUntilChanged, switchMap } from "rxjs/operators";
 import { AppSettings } from "../../../common/app-setting";
-import { NotebookMigrationService, notebookMappingKey } from "../notebook-migration/notebook-migration.service";
+import {
+  NotebookMigrationService,
+  notebookMappingKey,
+  notebookFileName,
+} from "../notebook-migration/notebook-migration.service";
 import { GuiConfigService } from "../../../common/service/gui-config.service";
 
 @Injectable({
@@ -139,7 +143,11 @@ export class JupyterPanelService {
         if (response.exists) {
           this.notebookMigrationService.setMapping(notebookMappingKey(workflowID), response.mapping);
 
-          if ((await this.notebookMigrationService.sendNotebookToJupyter(response.notebook)) == 1) {
+          const sent = await this.notebookMigrationService.sendNotebookToJupyter(
+            response.notebook,
+            notebookFileName(workflowID)
+          );
+          if (sent == 1) {
             return 1;
           } else {
             return 0;
@@ -205,6 +213,17 @@ export class JupyterPanelService {
     this.iframeRef = iframe;
   }
 
+  // Notebook filename for the workflow currently shown, used by the iframe fetch.
+  private currentNotebookFileName(): string {
+    return notebookFileName(this.workflowActionService.getWorkflow().wid);
+  }
+
+  // Iframe URL for the current workflow's notebook
+  public getJupyterIframeURLForWorkflow(): Promise<string | null> {
+    if (!this.enabled) return Promise.resolve(null);
+    return this.notebookMigrationService.getJupyterIframeURL(this.currentNotebookFileName());
+  }
+
   // Open the Jupyter Notebook panel
   public openPanel(panelName: string): void {
     if (!this.enabled) return;

frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts

@@ -18,7 +18,7 @@
  */
 
 import { TestBed } from "@angular/core/testing";
-import { NotebookMigrationService, notebookMappingKey } from "./notebook-migration.service";
+import { NotebookMigrationService, notebookMappingKey, notebookFileName } from "./notebook-migration.service";
 import { HttpClient } from "@angular/common/http";
 import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing";
 import { NotificationService } from "src/app/common/service/notification/notification.service";
@@ -100,11 +100,12 @@ describe("NotebookMigrationService", () => {
   it("should send notebook successfully and return 1", async () => {
     const mockNotebook: any = { cells: [] };
 
-    const promise = service.sendNotebookToJupyter(mockNotebook);
+    const promise = service.sendNotebookToJupyter(mockNotebook, "notebook_1.ipynb");
 
     const req = httpMock.expectOne(req => req.url.includes("/notebook-migration/set-notebook"));
 
     expect(req.request.method).toBe("POST");
+    expect(req.request.body.notebookName).toBe("notebook_1.ipynb");
 
     req.flush({ success: true });
 
@@ -117,7 +118,7 @@ describe("NotebookMigrationService", () => {
   it("should handle error when sending notebook and return 0", async () => {
     const mockNotebook: any = { cells: [] };
 
-    const promise = service.sendNotebookToJupyter(mockNotebook);
+    const promise = service.sendNotebookToJupyter(mockNotebook, "notebook_1.ipynb");
 
     const req = httpMock.expectOne(req => req.url.includes("/notebook-migration/set-notebook"));
 
@@ -135,7 +136,7 @@ describe("NotebookMigrationService", () => {
     // Error` branch. No request reaches the testing backend, so verify() stays happy.
     vi.spyOn(TestBed.inject(HttpClient), "post").mockReturnValue(throwError(() => new Error("network down")));
 
-    const result = await service.sendNotebookToJupyter({ cells: [] } as any);
+    const result = await service.sendNotebookToJupyter({ cells: [] } as any, "notebook_1.ipynb");
 
     expect(result).toBe(0);
     expect(mockNotificationService.error).toHaveBeenCalledWith(expect.stringContaining("network down"));
@@ -175,6 +176,18 @@ describe("NotebookMigrationService", () => {
 
     const req = httpMock.expectOne(req => req.url.includes("/notebook-migration/get-jupyter-iframe-url"));
     expect(req.request.method).toBe("GET");
+    // No name given, so no notebookName query param is sent.
+    expect(req.request.params.has("notebookName")).toBe(false);
+    req.flush({ success: true, url: "http://iframe" });
+
+    expect(await promise).toBe("http://iframe");
+  });
+
+  it("sends the notebookName as a query param when one is given", async () => {
+    const promise = service.getJupyterIframeURL("notebook_1.ipynb");
+
+    const req = httpMock.expectOne(req => req.url.includes("/notebook-migration/get-jupyter-iframe-url"));
+    expect(req.request.params.get("notebookName")).toBe("notebook_1.ipynb");
     req.flush({ success: true, url: "http://iframe" });
 
     expect(await promise).toBe("http://iframe");
@@ -235,6 +248,11 @@ describe("NotebookMigrationService", () => {
     expect(notebookMappingKey(42)).toBe("mapping_wid_42");
   });
 
+  it("notebookFileName builds a per-workflow filename from the wid, defaulting when absent", () => {
+    expect(notebookFileName(42)).toBe("notebook_42.ipynb");
+    expect(notebookFileName(undefined)).toBe("notebook.ipynb");
+  });
+
   // deleteNotebookAndMapping
   it("should call deleteNotebookAndMapping API with the wid", () => {
     let result: any;
@@ -318,7 +336,7 @@ describe("NotebookMigrationService", () => {
     });
 
     it("sendNotebookToJupyter returns 0 with no HTTP call or notification", async () => {
-      const result = await service.sendNotebookToJupyter({ cells: [] } as any);
+      const result = await service.sendNotebookToJupyter({ cells: [] } as any, "notebook_1.ipynb");
       expect(result).toBe(0);
       expect(mockNotificationService.success).not.toHaveBeenCalled();
       expect(mockNotificationService.error).not.toHaveBeenCalled();

frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts

@@ -61,6 +61,12 @@ export function notebookMappingKey(wid: number | undefined): string {
   return "mapping_wid_" + wid;
 }
 
+// Per-workflow notebook filename so workflows don't overwrite each other's notebook.
+// Falls back to the default when there's no wid.
+export function notebookFileName(wid: number | undefined): string {
+  return wid ? `notebook_${wid}.ipynb` : "notebook.ipynb";
+}
+
 @Injectable({
   providedIn: "root",
 })
@@ -132,16 +138,12 @@ export class NotebookMigrationService {
     return new NotebookMigrationLLM(this.config, this.workflowUtilService);
   }
 
-  public async sendNotebookToJupyter(notebookData: Notebook) {
+  public async sendNotebookToJupyter(notebookData: Notebook, notebookName: string) {
     if (!this.enabled) return 0;
     const jupyterAPIUrl = `${AppSettings.getApiEndpoint()}/notebook-migration/set-notebook`;
 
     const requestBody = {
-      // Fixed filename is intentional for the v1 per-user-pod design: each user runs
-      // their own notebook-migration-service and Jupyter, so a single notebook.ipynb
-      // never collides. A shared multi-user (global) service would need per-user or
-      // per-workflow keying here and for the backend's process-global jupyterIframeURL.
-      notebookName: "notebook.ipynb",
+      notebookName: notebookName,
       notebookData: notebookData,
     };
 
@@ -182,14 +184,16 @@ export class NotebookMigrationService {
     }
   }
 
-  public async getJupyterIframeURL(): Promise<string | null> {
+  public async getJupyterIframeURL(notebookName?: string): Promise<string | null> {
     if (!this.enabled) return null;
     try {
-      const data = await firstValueFrom(
-        this.http.get<{ success: boolean; url?: string }>(
-          `${AppSettings.getApiEndpoint()}/notebook-migration/get-jupyter-iframe-url`
-        )
-      );
+      const url = `${AppSettings.getApiEndpoint()}/notebook-migration/get-jupyter-iframe-url`;
+      // Send notebookName when given; otherwise the backend uses its default.
+      const params: Record<string, string> = {};
+      if (notebookName) {
+        params["notebookName"] = notebookName;
+      }
+      const data = await firstValueFrom(this.http.get<{ success: boolean; url?: string }>(url, { params }));
 
       if (!data.success || !data.url) {
         console.error("Jupyter server unavailable");