Harden e2e test setup: vault selection, page detection, and review fixes

- Fix vault selection by manipulating obsidian.json instead of unsupported --vault flag
- Find correct workspace page via DOM query instead of assuming pages()[0]
- Load .env via dotenv for OBSIDIAN_TEST_VAULT configuration
- Replace blind waitForTimeout calls with proper selector/function waits
- Add ensureVaultWithPlugin for safe setup on existing vaults
- Add CDP connection retry logic
- Scope eslint-disables to individual page.evaluate blocks
- Use crypto.randomBytes for proper hex vault IDs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: trangdoan982

apps/obsidian/e2e/helpers/commands.ts

@@ -8,11 +8,12 @@ export const executeCommand = async (
   page: Page,
   commandId: string,
 ): Promise<void> => {
+  /* eslint-disable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access */
   await page.evaluate((id) => {
     // @ts-expect-error - Obsidian's global `app` is available at runtime
     app.commands.executeCommandById(`@discourse-graph/obsidian:${id}`);
   }, commandId);
-  await page.waitForTimeout(500);
+  /* eslint-enable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access */
 };
 
 /**
@@ -24,20 +25,24 @@ export const executeCommandViaPalette = async (
   commandLabel: string,
 ): Promise<void> => {
   await page.keyboard.press("Meta+p");
-  await page.waitForTimeout(500);
+  await page.waitForSelector(".prompt-input", { timeout: 10_000 });
 
   await page.keyboard.type(commandLabel, { delay: 30 });
-  await page.waitForTimeout(500);
+  await page.waitForSelector(".suggestion-item", { timeout: 10_000 });
 
   await page.keyboard.press("Enter");
-  await page.waitForTimeout(1_000);
+  await page.waitForSelector(".prompt-container", {
+    state: "hidden",
+    timeout: 10_000,
+  });
 };
 
 /**
  * Ensure an active editor exists by creating and opening a scratch file.
  * Required before running editorCallback commands like "Create discourse node".
  */
 export const ensureActiveEditor = async (page: Page): Promise<void> => {
+  /* eslint-disable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */
   await page.evaluate(async () => {
     // @ts-expect-error - Obsidian's global `app` is available at runtime
     const vault = app.vault;
@@ -46,5 +51,6 @@ export const ensureActiveEditor = async (page: Page): Promise<void> => {
     // @ts-expect-error - Obsidian's global `app` is available at runtime
     await app.workspace.openLinkText(file.path, "", false);
   });
-  await page.waitForTimeout(1_000);
+  /* eslint-enable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */
+  await page.waitForSelector(".cm-editor", { timeout: 10_000 });
 };

apps/obsidian/e2e/helpers/obsidian-setup.ts

@@ -1,56 +1,174 @@
+import crypto from "crypto";
 import fs from "fs";
+import os from "os";
 import path from "path";
 import { spawn, execSync, type ChildProcess } from "child_process";
 import { chromium, type Browser, type Page } from "@playwright/test";
 
 const OBSIDIAN_PATH = "/Applications/Obsidian.app/Contents/MacOS/Obsidian";
+const OBSIDIAN_CONFIG_PATH = path.join(
+  os.homedir(),
+  "Library",
+  "Application Support",
+  "obsidian",
+  "obsidian.json",
+);
 const PLUGIN_ID = "@discourse-graph/obsidian";
 const DEBUG_PORT = 9222;
 
-export const createTestVault = (vaultPath: string): void => {
-  // Clean up any existing vault
-  cleanTestVault(vaultPath);
+/**
+ * Returns the vault path to use for tests.
+ * Reads from OBSIDIAN_TEST_VAULT env var, or falls back to the provided default.
+ */
+export const resolveVaultPath = (defaultPath: string): string =>
+  // eslint-disable-next-line turboPlugin/no-undeclared-env-vars -- local dev-only, not a turbo cache key
+  process.env.OBSIDIAN_TEST_VAULT ?? defaultPath;
 
-  // Create vault directories
+/**
+ * Whether we're using a custom vault (skip create/clean).
+ */
+export const isCustomVault = (): boolean =>
+  // eslint-disable-next-line turboPlugin/no-undeclared-env-vars -- local dev-only, not a turbo cache key
+  process.env.OBSIDIAN_TEST_VAULT !== undefined;
+
+/**
+ * Ensure a vault exists at vaultPath with the plugin installed.
+ *
+ * Safe to call on existing vaults — only creates directories and
+ * copies plugin files without wiping existing vault content.
+ * For fresh temp vaults, call cleanTestVault first.
+ */
+export const ensureVaultWithPlugin = (vaultPath: string): void => {
   const obsidianDir = path.join(vaultPath, ".obsidian");
   const pluginDir = path.join(obsidianDir, "plugins", PLUGIN_ID);
   fs.mkdirSync(pluginDir, { recursive: true });
 
-  // Write community-plugins.json to enable our plugin
-  fs.writeFileSync(
-    path.join(obsidianDir, "community-plugins.json"),
-    JSON.stringify([PLUGIN_ID]),
-  );
+  // Ensure community-plugins.json includes our plugin
+  const communityPluginsPath = path.join(obsidianDir, "community-plugins.json");
+  let enabledPlugins: string[] = [];
+  if (fs.existsSync(communityPluginsPath)) {
+    enabledPlugins = JSON.parse(
+      fs.readFileSync(communityPluginsPath, "utf-8"),
+    ) as string[];
+  }
+  if (!enabledPlugins.includes(PLUGIN_ID)) {
+    enabledPlugins.push(PLUGIN_ID);
+    fs.writeFileSync(communityPluginsPath, JSON.stringify(enabledPlugins));
+  }
 
-  // Write app.json to disable restricted mode (allows community plugins)
-  fs.writeFileSync(
-    path.join(obsidianDir, "app.json"),
-    JSON.stringify({ livePreview: true }),
-  );
+  // Ensure app.json exists
+  const appJsonPath = path.join(obsidianDir, "app.json");
+  if (!fs.existsSync(appJsonPath)) {
+    fs.writeFileSync(appJsonPath, JSON.stringify({ livePreview: true }));
+  }
 
-  // Copy built plugin files from dist/ into the vault's plugin directory
+  // Copy built plugin files from dist/
   const distDir = path.join(__dirname, "..", "..", "dist");
   if (!fs.existsSync(distDir)) {
     throw new Error(
       `dist/ directory not found at ${distDir}. Run "pnpm build" first.`,
     );
   }
 
-  const filesToCopy = ["main.js", "manifest.json", "styles.css"];
-  for (const file of filesToCopy) {
+  for (const file of ["main.js", "manifest.json", "styles.css"]) {
     const src = path.join(distDir, file);
     if (fs.existsSync(src)) {
       fs.copyFileSync(src, path.join(pluginDir, file));
     }
   }
 };
 
+/**
+ * Create a fresh temp vault (wipes existing content).
+ * Only use this for auto-created temp vaults, not custom vaults.
+ */
+export const createTestVault = (vaultPath: string): void => {
+  cleanTestVault(vaultPath);
+  ensureVaultWithPlugin(vaultPath);
+};
+
 export const cleanTestVault = (vaultPath: string): void => {
   if (fs.existsSync(vaultPath)) {
     fs.rmSync(vaultPath, { recursive: true, force: true });
   }
 };
 
+/**
+ * Set the target vault as the active vault in Obsidian's config.
+ *
+ * Obsidian doesn't support a `--vault` CLI flag. Instead, it opens
+ * whichever vault has `"open": true` in obsidian.json. We manipulate
+ * that file before launch and restore it after tests.
+ */
+const setActiveVault = (vaultPath: string): string | undefined => {
+  if (!fs.existsSync(OBSIDIAN_CONFIG_PATH)) return undefined;
+
+  const original = fs.readFileSync(OBSIDIAN_CONFIG_PATH, "utf-8");
+  const config = JSON.parse(original) as {
+    vaults: Record<string, { path: string; ts: number; open?: boolean }>;
+  };
+
+  let targetId: string | undefined;
+  for (const [id, vault] of Object.entries(config.vaults)) {
+    delete vault.open;
+    if (vault.path === vaultPath) {
+      targetId = id;
+    }
+  }
+
+  if (!targetId) {
+    targetId = crypto.randomBytes(8).toString("hex");
+    config.vaults[targetId] = { path: vaultPath, ts: Date.now() };
+  }
+
+  const targetVault = config.vaults[targetId]!;
+  targetVault.open = true;
+  targetVault.ts = Date.now();
+
+  fs.writeFileSync(OBSIDIAN_CONFIG_PATH, JSON.stringify(config));
+  return original;
+};
+
+/**
+ * Restore the original obsidian.json after tests.
+ */
+export const restoreObsidianConfig = (original: string): void => {
+  fs.writeFileSync(OBSIDIAN_CONFIG_PATH, original);
+};
+
+/**
+ * Find the vault workspace page among all CDP pages.
+ *
+ * Obsidian opens multiple windows (dev console, main workspace).
+ * We need the one that has the `.workspace` element.
+ */
+const findWorkspacePage = async (
+  browser: Browser,
+  timeout = 30_000,
+): Promise<Page> => {
+  const start = Date.now();
+  while (Date.now() - start < timeout) {
+    for (const context of browser.contexts()) {
+      for (const page of context.pages()) {
+        const hasWorkspace = await page
+          .locator(".workspace")
+          .first()
+          .isVisible()
+          .catch(() => false);
+        if (hasWorkspace) return page;
+      }
+    }
+    await new Promise((resolve) => setTimeout(resolve, 500));
+  }
+  throw new Error(
+    `No page with .workspace found within ${timeout}ms. ` +
+      `Available pages: ${browser
+        .contexts()
+        .flatMap((c) => c.pages().map((p) => p.url()))
+        .join(", ")}`,
+  );
+};
+
 /**
  * Launch Obsidian as a subprocess with remote debugging enabled,
  * then connect via Chrome DevTools Protocol.
@@ -65,20 +183,30 @@ export const launchObsidian = async (
   browser: Browser;
   page: Page;
   obsidianProcess: ChildProcess;
+  originalObsidianConfig?: string;
 }> => {
-  // Kill any existing Obsidian instances to free the debug port
+  // Kill only the process currently holding the debug port
   try {
-    execSync("pkill -f Obsidian", { stdio: "ignore" });
-    // Wait for processes to fully exit
-    await new Promise((resolve) => setTimeout(resolve, 2_000));
+    const pids = execSync(`lsof -ti tcp:${DEBUG_PORT}`, {
+      stdio: ["ignore", "pipe", "ignore"],
+    })
+      .toString()
+      .trim();
+    if (pids) {
+      execSync(`kill -9 ${pids.split("\n").join(" ")}`, { stdio: "ignore" });
+      await new Promise((resolve) => setTimeout(resolve, 500));
+    }
   } catch {
-    // No existing Obsidian processes — that's fine
+    // No process on the port — that's fine
   }
 
+  // Set the target vault as active in Obsidian's config before launching
+  const originalObsidianConfig = setActiveVault(vaultPath);
+
   // Launch Obsidian with remote debugging port
   const obsidianProcess = spawn(
     OBSIDIAN_PATH,
-    [`--remote-debugging-port=${DEBUG_PORT}`, `--vault=${vaultPath}`],
+    [`--remote-debugging-port=${DEBUG_PORT}`],
     {
       stdio: "pipe",
       detached: true,
@@ -92,33 +220,38 @@ export const launchObsidian = async (
     console.log("[Obsidian stdout]", data.toString());
   });
 
-  // Unref so the spawned process doesn't keep the test runner alive
   obsidianProcess.unref();
 
   // Wait for the debug port to be ready
   await waitForDebugPort(DEBUG_PORT, 30_000);
 
-  // Connect to Obsidian via CDP
-  const browser = await chromium.connectOverCDP(
-    `http://localhost:${DEBUG_PORT}`,
-  );
-
-  // Get the first browser context and page
-  const context = browser.contexts()[0];
-  if (!context) {
-    throw new Error("No browser context found after connecting via CDP");
-  }
+  // Connect to Obsidian via CDP with retry logic
+  const browser = await connectWithRetry(`http://localhost:${DEBUG_PORT}`);
 
-  let page = context.pages()[0];
-  if (!page) {
-    page = await context.waitForEvent("page");
-  }
+  // Find the workspace page (not the dev console or other windows)
+  const page = await findWorkspacePage(browser);
 
-  // Wait for Obsidian to finish initializing
-  await page.waitForLoadState("domcontentloaded");
-  await page.waitForSelector(".workspace", { timeout: 30_000 });
+  return { browser, page, obsidianProcess, originalObsidianConfig };
+};
 
-  return { browser, page, obsidianProcess };
+const connectWithRetry = async (
+  url: string,
+  maxRetries = 5,
+  delayMs = 1_000,
+): Promise<Browser> => {
+  let lastError: unknown;
+  for (let i = 0; i < maxRetries; i++) {
+    try {
+      return await chromium.connectOverCDP(url);
+    } catch (e) {
+      lastError = e;
+      console.log(
+        `CDP connect attempt ${i + 1}/${maxRetries} failed, retrying in ${delayMs}ms...`,
+      );
+      await new Promise((resolve) => setTimeout(resolve, delayMs));
+    }
+  }
+  throw lastError;
 };
 
 const waitForDebugPort = async (