fix: add retry to server start, only start one server at a time, open…

… terminal by default (#76)

README.md

@@ -113,7 +113,7 @@ To ensure marimo works correctly with your Python environment, you have several
 - `marimo.https`: Enable HTTPS for marimo server (default: `false`)
 - `marimo.enableToken`: Enable token authentication (default: `false`)
 - `marimo.tokenPassword`: Token password (default: _empty_)
-- `marimo.showTerminal`: Open the terminal when the server starts (default: `false`)
+- `marimo.showTerminal`: Open the terminal when the server starts (default: `true`)
 - `marimo.debug`: Enable debug logging (default: `false`)
 - `marimo.pythonPath`: Path to python interpreter (default: the one from python extension). Will be used with `/path/to/python -m marimo` to invoke marimo.
 - `marimo.marimoPath`: Path to a marimo executable (default: None). This will override use of the `pythonPath` setting, and instead invoke commands like `/path/to/marimo edit` instead of `python -m marimo edit`.

package.json

@@ -10,27 +10,17 @@
     "vscode": "^1.95.0"
   },
   "private": true,
-  "categories": [
-    "Notebooks",
-    "Machine Learning",
-    "Data Science"
-  ],
+  "categories": ["Notebooks", "Machine Learning", "Data Science"],
   "icon": "resources/marimo.png",
-  "activationEvents": [
-    "onLanguage"
-  ],
+  "activationEvents": ["onLanguage"],
   "repository": {
     "type": "git",
     "url": "https://github.com/marimo-team/vscode-marimo"
   },
   "bugs": {
     "url": "https://github.com/marimo-team/vscode-marimo/issues"
   },
-  "files": [
-    "dist",
-    "resources",
-    "LICENSE"
-  ],
+  "files": ["dist", "resources", "LICENSE"],
   "main": "./dist/extension.js",
   "contributes": {
     "commands": [
@@ -327,10 +317,7 @@
       "properties": {
         "marimo.browserType": {
           "type": "string",
-          "enum": [
-            "embedded",
-            "system"
-          ],
+          "enum": ["embedded", "system"],
           "default": "embedded",
           "description": "The type of browser to use for opening marimo apps."
         },
@@ -395,7 +382,7 @@
   "scripts": {
     "build": "tsup src/extension.ts --external=vscode",
     "dev": "pnpm run build --watch",
-    "lint": "biome check --apply .",
+    "lint": "biome check --write .",
     "pack": "vsce package --no-dependencies",
     "codegen": "npx openapi-typescript https://raw.githubusercontent.com/marimo-team/marimo/0.8.22/openapi/api.yaml --immutable -o ./src/generated/api.ts",
     "publish": "vsce publish --no-dependencies",

src/__integration__/server-manager.test.ts

@@ -12,6 +12,8 @@ import { setExtension } from "../ctx";
 import { ServerManager } from "../services/server-manager";
 import { sleep } from "../utils/async";
 
+const TEST_TIMEOUT = 10_000;
+
 describe("ServerManager integration tests", () => {
   let config: Config;
   let manager: ServerManager;
@@ -50,82 +52,141 @@ describe("ServerManager integration tests", () => {
     await manager.stopServer();
   });
 
-  it("should start and stop server", async () => {
-    expect(manager.getStatus()).toBe("stopped");
-
-    const result = await manager.start();
-    expect(result.port).toBeGreaterThan(0);
-    expect(result.skewToken).toBeDefined();
-    expect(result.version).toMatch(/\d+\.\d+\.\d+/);
-    expect(manager.getStatus()).toBe("started");
-
-    await manager.stopServer();
-    expect(manager.getStatus()).toBe("stopped");
-  });
-
-  it("should start and stop a server with custom python path", async () => {
-    workspace.getConfiguration().set("marimo.pythonPath", "uv run python");
-    const result = await manager.start();
-    expect(result.port).toBeGreaterThan(0);
-    expect(result.skewToken).toBeDefined();
-    expect(result.version).toMatch(/\d+\.\d+\.\d+/);
-    expect(manager.getStatus()).toBe("started");
-
-    await manager.stopServer();
-    expect(manager.getStatus()).toBe("stopped");
-  });
-
-  it("should start and stop a server with custom marimo path", async () => {
-    workspace.getConfiguration().set("marimo.marimoPath", "uv run marimo");
-    const result = await manager.start();
-    expect(result.port).toBeGreaterThan(0);
-    expect(result.skewToken).toBeDefined();
-    expect(result.version).toMatch(/\d+\.\d+\.\d+/);
-    expect(manager.getStatus()).toBe("started");
-
-    await manager.stopServer();
-    expect(manager.getStatus()).toBe("stopped");
-  });
-
-  it("should reuse existing server if healthy", async () => {
-    const firstStart = await manager.start();
-    const secondStart = await manager.start();
-
-    expect(secondStart.port).toBe(firstStart.port);
-    expect(secondStart.skewToken).toBe(firstStart.skewToken);
-
-    // Check if healthy
-    const isHealthy = await manager.isHealthy(firstStart.port);
-    expect(isHealthy).toBe(true);
-  });
-
-  it.skip("should restart unhealthy server", async () => {
-    const firstStart = await manager.start();
-
-    // Force server into unhealthy state by stopping it directly
-    await manager.stopServer();
-
-    // Should detect unhealthy state and restart
-    const secondStart = await manager.start();
-    expect(secondStart.port).not.toBe(firstStart.port);
-  });
-
-  it("should handle multiple start requests while starting", async () => {
-    const startPromise1 = manager.start();
-    const startPromise2 = manager.start();
-
-    const [result1, result2] = await Promise.all([
-      startPromise1,
-      startPromise2,
-    ]);
-    expect(result1.port).toBe(result2.port);
-  });
-
-  it("should get active sessions", async () => {
-    await manager.start();
-    const sessions = await manager.getActiveSessions();
-    expect(Array.isArray(sessions)).toBe(true);
-  });
+  it(
+    "should start and stop server",
+    async () => {
+      expect(manager.getStatus()).toBe("stopped");
+
+      const result = await manager.start();
+      expect(result.port).toBeGreaterThan(0);
+      expect(result.skewToken).toBeDefined();
+      expect(result.version).toMatch(/\d+\.\d+\.\d+/);
+      expect(manager.getStatus()).toBe("started");
+
+      await manager.stopServer();
+      expect(manager.getStatus()).toBe("stopped");
+    },
+    TEST_TIMEOUT,
+  );
+
+  it(
+    "should start and stop a server with custom python path",
+    async () => {
+      workspace.getConfiguration().set("marimo.pythonPath", "uv run python");
+      const result = await manager.start();
+      expect(result.port).toBeGreaterThan(0);
+      expect(result.skewToken).toBeDefined();
+      expect(result.version).toMatch(/\d+\.\d+\.\d+/);
+      expect(manager.getStatus()).toBe("started");
+
+      await manager.stopServer();
+      expect(manager.getStatus()).toBe("stopped");
+    },
+    TEST_TIMEOUT,
+  );
+
+  it(
+    "should start and stop a server with custom marimo path",
+    async () => {
+      workspace.getConfiguration().set("marimo.marimoPath", "uv run marimo");
+      const result = await manager.start();
+      expect(result.port).toBeGreaterThan(0);
+      expect(result.skewToken).toBeDefined();
+      expect(result.version).toMatch(/\d+\.\d+\.\d+/);
+      expect(manager.getStatus()).toBe("started");
+
+      await manager.stopServer();
+      expect(manager.getStatus()).toBe("stopped");
+    },
+    TEST_TIMEOUT,
+  );
+
+  it(
+    "should reuse existing server if healthy",
+    async () => {
+      console.warn("Starting server");
+      const firstStart = await manager.start();
+      console.warn("Starting server again");
+      const secondStart = await manager.start();
+
+      expect(secondStart.port).toBe(firstStart.port);
+      expect(secondStart.skewToken).toBe(firstStart.skewToken);
+
+      // Check if healthy
+      const isHealthy = await manager.isHealthy(firstStart.port);
+      expect(isHealthy).toBe(true);
+    },
+    TEST_TIMEOUT,
+  );
+
+  it.skip(
+    "should start a server and cancel it",
+    async () => {
+      const subscribers = new Set<() => void>();
+      const token = {
+        isCancellationRequested: false,
+        onCancellationRequested: vi.fn().mockImplementation((fn) => {
+          subscribers.add(fn);
+        }),
+        cancel: vi.fn().mockImplementation(() => {
+          subscribers.forEach(async (fn) => {
+            try {
+              fn();
+            } catch (e) {
+              // pass
+            }
+          });
+        }),
+      };
+
+      await expect(async () => {
+        await Promise.all([manager.start(token), token.cancel(), sleep(100)]);
+      }).rejects.toThrow("Server start was cancelled");
+      expect(token.onCancellationRequested).toHaveBeenCalledTimes(1);
+      expect(manager.getStatus()).toBe("stopped");
+    },
+    TEST_TIMEOUT,
+  );
+
+  it(
+    "should restart unhealthy server",
+    async () => {
+      const firstStart = await manager.start();
+
+      // Force server into unhealthy state by stopping it directly
+      await manager.stopServer();
+
+      // Should detect unhealthy state and restart
+      const secondStart = await manager.start();
+      expect(secondStart.port).not.toBe(firstStart.port);
+    },
+    TEST_TIMEOUT,
+  );
+
+  it(
+    "should handle multiple start requests while starting",
+    async () => {
+      const startPromise1 = manager.start();
+      const startPromise2 = manager.start();
+
+      const [result1, result2] = await Promise.all([
+        startPromise1,
+        startPromise2,
+      ]);
+      expect(result1.port).toBe(result2.port);
+    },
+    TEST_TIMEOUT,
+  );
+
+  it(
+    "should get active sessions",
+    async () => {
+      await manager.start();
+      const sessions = await manager.getActiveSessions();
+      expect(Array.isArray(sessions)).toBe(true);
+    },
+    TEST_TIMEOUT,
+  );
 
   it.skip("should show warning and handle restart on unhealthy server", async () => {
     await manager.start();

src/__mocks__/vscode.ts

@@ -1,35 +1,48 @@
-import { spawn } from "node:child_process";
+import { type ChildProcess, spawn } from "node:child_process";
 import { createVSCodeMock as create } from "jest-mock-vscode";
-import type { VitestUtils } from "vitest";
+import { type VitestUtils, afterAll } from "vitest";
 import type { NotebookController } from "vscode";
 
-export function createVSCodeMock(vi: VitestUtils) {
+const openProcesses: ChildProcess[] = [];
+
+export async function createVSCodeMock(vi: VitestUtils) {
   // biome-ignore lint/suspicious/noExplicitAny: any is ok
   const vscode = create(vi) as any;
 
-  vscode.window.createWebviewPanel.mockImplementation(() => {
-    return {
-      webview: {
-        html: "",
-        onDidReceiveMessage: vi.fn(),
-      },
-      onDidDispose: vi.fn(),
-    };
-  });
-
   vscode.workspace = vscode.workspace || {};
   let configMap: Record<string, unknown> = {};
 
   // Add createTerminal mock
   vscode.window.createTerminal = vi.fn().mockImplementation(() => {
+    let proc: ChildProcess | undefined;
     return {
       processId: Promise.resolve(1),
-      dispose: vi.fn(),
+      dispose: vi.fn().mockImplementation(() => {
+        proc?.kill();
+      }),
       sendText: vi.fn().mockImplementation((args: string) => {
-        const proc = spawn(args, { shell: true });
-        proc.on("data", (data) => {
-          console.log(data.toString());
+        proc = spawn(args, { shell: true });
+        proc.stdout?.on("data", (data) => {
+          const line = data.toString();
+          if (line) {
+            console.warn(line);
+          }
+        });
+        proc.stderr?.on("data", (data) => {
+          const line = data.toString();
+          if (line) {
+            console.warn(line);
+          }
         });
+        proc.on("error", (error) => {
+          if (error) {
+            console.warn(error);
+          }
+        });
+        proc.on("close", (code) => {
+          console.warn(`Process exited with code ${code}`);
+        });
+        openProcesses.push(proc);
       }),
       show: vi.fn(),
     };
@@ -67,6 +80,16 @@ export function createVSCodeMock(vi: VitestUtils) {
     Default = 0,
   }
   vscode.QuickPickItemKind = QuickPickItemKind;
+  vscode.window.createWebviewPanel = vi.fn().mockImplementation(() => {
+    return {
+      webview: {
+        onDidReceiveMessage: vi.fn(),
+        html: "",
+      },
+      onDidDispose: vi.fn(),
+      dispose: vi.fn(),
+    };
+  });
 
   vscode.notebooks = vscode.notebooks || {};
   vscode.notebooks.createNotebookController = vi
@@ -90,3 +113,7 @@ export function createVSCodeMock(vi: VitestUtils) {
 
   return vscode;
 }
+
+afterAll(() => {
+  openProcesses.forEach((proc) => proc.kill());
+});

src/browser/__tests__/panel.test.ts

@@ -1,11 +1,12 @@
 import { describe, expect, it, vi } from "vitest";
 import { createVSCodeMock } from "../../__mocks__/vscode";
-import { MarimoPanelManager } from "../panel";
 
 vi.mock("vscode", async () => {
   return createVSCodeMock(vi);
 });
 
+import { MarimoPanelManager } from "../panel";
+
 describe("Panel", () => {
   it("should be created", async () => {
     const panel = new MarimoPanelManager("app");