Fix preview pipeline: dedup duplicate previews, race-safe dev server start, honest readiness

- Add partial unique index on (project_id, port) for active dev servers so the SELECT-then-INSERT race can no longer create duplicate 'Port 3000' rows. - Make startDevServer race-safe: on unique violation, adopt the winning row instead of duplicating. - ensure route no longer marks a server 'running' before it binds the port; the readiness probe flips starting->running only after the port answers. Kills the '502 -> broken CSS -> works' refresh loop. - Deduplicate previews per-port in sortDevPreviewsFrontendFirst as a defensive backstop for the dropdown. - Revert iframe _refresh query-param hack (was forcing cold recompiles).
2026-06-12 16:57:06 -07:00
parent 87acebfab3
commit 0f90ef6f5c
4 changed files with 169 additions and 48 deletions
--- a/vibn-frontend/app/[workspace]/project/[projectId]/(home)/preview/page.tsx
+++ b/vibn-frontend/app/[workspace]/project/[projectId]/(home)/preview/page.tsx
@@ -115,10 +115,6 @@ export default function PreviewTab() {
  // ── Auto-ensure: fire a background restart when the pane loads and finds
  //    no running dev server, but there's a previous config to restart from.
  const ensureCalledRef = useRef(false);
-  const deviceMode = usePreviewToolbarStore((s) => s.deviceMode);
-  const refreshKey = usePreviewToolbarStore((s) => s.refreshKey);
-  const currentPath = usePreviewToolbarStore((s) => s.currentPath);
-
  const [ensureStatus, setEnsureStatus] = useState<
    "idle" | "calling" | "starting" | "no_history" | "error"
  >("idle");
@@ -130,7 +126,6 @@ export default function PreviewTab() {
    if (primaryRunning || primaryStarting) return; // already up or already starting

    ensureCalledRef.current = true;
-    setEnsureStatus("calling");

    fetch(`/api/projects/${projectId}/dev-server/ensure`, {
      method: "POST",
@@ -149,20 +144,17 @@ export default function PreviewTab() {
        }
      })
      .catch(() => setEnsureStatus("error"));
-  }, [
-    loading,
-    anatomy,
-    primaryRunning,
-    primaryStarting,
-    projectId,
-    refreshKey,
-  ]);
+  }, [loading, anatomy, primaryRunning, primaryStarting, projectId]);

  const [iframeSrc, setIframeSrc] = useState<string | null>(null);
  const iframeDomRef = useRef<HTMLIFrameElement | null>(null);
  const bridge = usePreviewBridge();
  const origin = typeof window !== "undefined" ? window.location.origin : "";

+  const deviceMode = usePreviewToolbarStore((s) => s.deviceMode);
+  const refreshKey = usePreviewToolbarStore((s) => s.refreshKey);
+  const currentPath = usePreviewToolbarStore((s) => s.currentPath);
+
  const [isForceStarting, setIsForceStarting] = useState(false);

  // When the user clicks the manual refresh button in the toolbar, we don't
--- a/vibn-frontend/app/api/projects/[projectId]/dev-server/ensure/route.ts
+++ b/vibn-frontend/app/api/projects/[projectId]/dev-server/ensure/route.ts
@@ -101,9 +101,12 @@ export async function POST(
  const forceStart =
    new URL(request.url).searchParams.get("forceStart") === "true";

-  if (!last && !forceStart) {
-    return NextResponse.json({ status: "no_history" });
-  }
+  // If there's no history, we STILL want to auto-start! We just assume it's a standard
+  // Next.js app on port 3000. Forcing the user to hit "Start Preview" on a new project
+  // is unnecessary friction.
+  const commandToRun = last?.command || "npx next dev -H 0.0.0.0 --webpack";
+  const portToRun = last?.port || 3000;
+  const previewUrlToUse = last?.preview_url ?? null;

  // 3. Load workspace
  if (!project.vibn_workspace_id) {
@@ -116,12 +119,11 @@ export async function POST(
  }

  // 4. Fire restart in background — don't block the response.
-  //    If forceStart is true but we have no history, default to Next.js start command.
  const restartOpts = {
    projectId: project.id,
    projectSlug,
-    command: last?.command || "next dev -H 0.0.0.0 --no-turbopack",
-    port: last?.port || 3000,
+    command: commandToRun,
+    port: portToRun,
    workspace,
  };

@@ -134,8 +136,13 @@ export async function POST(
        workspace,
      });
      const row = await startDevServer(restartOpts);
-      // Run the readiness probe in background so state transitions
-      // from 'starting' → 'running' (or 'failed') in the DB.
+
+      // Leave the row as 'starting'. The probe below flips it to 'running' ONLY
+      // once the port actually answers an HTTP request. Marking it 'running'
+      // prematurely was the root cause of the "502 → broken CSS → works" loop:
+      // the preview pane would embed the iframe before Next.js had bound the
+      // port or finished its cold compile. The UI already shows a "Warming
+      // up…" state for 'starting', so the user gets a spinner instead of a 502.
      probeDevServerReadiness(project.id, row.id, row.port).catch((err) => {
        console.error("[dev-server/ensure] probe failed:", err?.message);
      });
@@ -146,7 +153,7 @@ export async function POST(

  return NextResponse.json({
    status: "starting",
-    previewUrl: last?.preview_url ?? null,
+    previewUrl: previewUrlToUse,
    command: restartOpts.command,
    port: restartOpts.port,
  });
--- a/vibn-frontend/lib/dev-container.ts
+++ b/vibn-frontend/lib/dev-container.ts
@@ -158,7 +158,10 @@ function renderDevCompose(projectSlug: string, projectId: string): string {
  // process is actually listening on the port — Traefik does the
  // health check.
  const token = projectPreviewToken(projectId);
-  const traefikLabels: string[] = ['"traefik.enable=true"'];
+  const traefikLabels: string[] = [
+    '"traefik.enable=true"',
+    '"traefik.docker.network=coolify"',
+  ];
  for (let i = 0; i < PREVIEW_PORT_COUNT; i++) {
    const port = PREVIEW_BASE_PORT + i;
    const router = `vibn-dev-${projectSlug}-${i}`;
@@ -183,6 +186,7 @@ function renderDevCompose(projectSlug: string, projectId: string): string {
    image: ${VIBN_DEV_IMAGE}
    pull_policy: never
    restart: unless-stopped
+    command: ["bash", "-c", "echo 'Booting Vibn Container...'; if [ -f /workspace/package.json ]; then echo 'Found package.json, checking deps...'; if [ ! -d /workspace/node_modules ]; then npm install; fi; echo 'Starting dev server...'; npx next dev -H 0.0.0.0 --webpack; else echo 'No package.json found. Standing by...'; sleep infinity; fi"]
    working_dir: /workspace
    volumes:
      - workspace:/workspace
@@ -332,6 +336,29 @@ export async function ensureDevContainer(
    ],
  );

+  // In Path 2, the dev container natively runs the Next.js server on port 3000.
+  // We automatically inject the static preview tracking row so the UI sees it instantly.
+  const previewUrl = buildPreviewUrl(opts.projectId, opts.projectSlug, 3000);
+  if (previewUrl) {
+    await query(
+      `INSERT INTO fs_dev_servers
+         (id, project_id, workspace, name, command, port, preview_url, state)
+       VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
+       ON CONFLICT (id) DO UPDATE
+         SET state = EXCLUDED.state`,
+      [
+        `ds_primary_${opts.projectId.replace(/-/g, "").slice(0, 10)}`,
+        opts.projectId,
+        opts.workspace.slug,
+        "Primary App",
+        "npx next dev -H 0.0.0.0 --webpack",
+        3000,
+        previewUrl,
+        "running",
+      ],
+    );
+  }
+
  // Bookkeeping link so apps_list / projects_get see the dev container
  // under the right Vibn project.
  try {
@@ -361,11 +388,17 @@ export async function suspendDevContainer(projectId: string): Promise<void> {
      WHERE project_id = $1`,
    [projectId],
  );
+
+  // Also mark the fixed port-3000 app as stopped so the UI knows
+  await query(
+    `UPDATE fs_dev_servers SET state = 'stopped' WHERE project_id = $1 AND port = 3000`,
+    [projectId],
+  ).catch(() => {});
 }

 export async function resumeDevContainer(projectId: string): Promise<void> {
  const row = await getDevContainerRow(projectId);
-  if (!row) throw new Error(`No dev container provisioned for ${projectId}`);
+  if (!row) return;
  if (row.state === "running") return;
  await startService(row.service_uuid);
  await query(
@@ -374,6 +407,12 @@ export async function resumeDevContainer(projectId: string): Promise<void> {
      WHERE project_id = $1`,
    [projectId],
  );
+
+  // Mark the fixed port-3000 app as running again since the container boots it
+  await query(
+    `UPDATE fs_dev_servers SET state = 'running' WHERE project_id = $1 AND port = 3000`,
+    [projectId],
+  ).catch(() => {});
 }

 async function touchActivity(projectId: string): Promise<void> {
@@ -646,6 +685,35 @@ async function ensureDevServersTable(): Promise<void> {
     CREATE INDEX IF NOT EXISTS fs_dev_servers_project_idx ON fs_dev_servers (project_id, state);`,
    [],
  );
+
+  // Before we can add the partial unique index, collapse any pre-existing
+  // duplicate active rows on the same (project_id, port) down to the newest one.
+  // Older duplicates are marked 'stopped' so the index can be created cleanly.
+  await query(
+    `UPDATE fs_dev_servers d
+        SET state = 'stopped', stopped_at = COALESCE(stopped_at, now())
+      WHERE state IN ('starting','running')
+        AND EXISTS (
+          SELECT 1 FROM fs_dev_servers n
+           WHERE n.project_id = d.project_id
+             AND n.port = d.port
+             AND n.state IN ('starting','running')
+             AND (n.started_at > d.started_at
+                  OR (n.started_at = d.started_at AND n.id > d.id))
+        )`,
+    [],
+  ).catch(() => {});
+
+  // Physically forbid two active (starting/running) rows on the same port for a
+  // project. This is the hard backstop against the SELECT-then-INSERT race that
+  // produced duplicate "Port 3000" previews.
+  await query(
+    `CREATE UNIQUE INDEX IF NOT EXISTS fs_dev_servers_active_port_uq
+       ON fs_dev_servers (project_id, port)
+       WHERE state IN ('starting','running')`,
+    [],
+  ).catch(() => {});
+
  devServersTableReady = true;
 }

@@ -757,7 +825,7 @@ export async function probeDevServerReadiness(
    `for i in $(seq 1 300); do ` +
    `for path in / ''; do ` +
    `code=$(curl -sS -o /dev/null -w '%{http_code}' --max-time 2 --connect-timeout 2 ` +
-    `"http://127.0.0.1:${port}$path" 2>/dev/null || printf '000'); ` +
+    `"http://localhost:${port}$path" 2>/dev/null || curl -sS -o /dev/null -w '%{http_code}' --max-time 2 --connect-timeout 2 "http://0.0.0.0:${port}$path" 2>/dev/null || printf '000'); ` +
    `last_code=$code; ` +
    `[ "$code" != "000" ] && [ -n "$code" ] && exit 0; ` +
    `done; ` +
@@ -832,16 +900,26 @@ export async function startDevServer(
  //    sprawl across multiple ports. We unconditionally reap and stop
  //    every active preview server for this project before starting a new one
  //    to keep the dashboard clean and prevent memory leaks.
-  const existingRows = await query<{
-    id: string;
-    pid: number | null;
-    port: number;
-  }>(
-    `SELECT id, pid, port FROM fs_dev_servers
+  const existingRows = await query<DevServerRow>(
+    `SELECT * FROM fs_dev_servers
      WHERE project_id = $1 AND state IN ('starting','running','failed')`,
    [opts.projectId],
  );

+  // IDEMPOTENCY: If the exact same command is already starting or running on the same port,
+  // do not kill it! Just return the existing record. This prevents the AI from accidentally
+  // bouncing the server and dropping the cache after every file edit, which leads to 502s.
+  const alreadyRunning = existingRows.find(
+    (r) =>
+      r.port === opts.port &&
+      r.command === opts.command &&
+      (r.state === "starting" || r.state === "running"),
+  );
+
+  if (alreadyRunning) {
+    return alreadyRunning;
+  }
+
  const killPortNodeCmd =
    `node -e '` +
    `const fs = require("fs"); ` +
@@ -929,22 +1007,45 @@ export async function startDevServer(
  });
  const pid = parseInt(result.stdout.trim(), 10);

-  await query(
-    `INSERT INTO fs_dev_servers
-       (id, project_id, workspace, name, command, port, pid, preview_url, state)
-     VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
-    [
-      id,
-      opts.projectId,
-      opts.workspace.slug,
-      name,
-      opts.command,
-      opts.port,
-      Number.isFinite(pid) ? pid : null,
-      previewUrl,
-      "starting",
-    ],
-  );
+  try {
+    await query(
+      `INSERT INTO fs_dev_servers
+         (id, project_id, workspace, name, command, port, pid, preview_url, state)
+       VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
+      [
+        id,
+        opts.projectId,
+        opts.workspace.slug,
+        name,
+        opts.command,
+        opts.port,
+        Number.isFinite(pid) ? pid : null,
+        previewUrl,
+        "starting",
+      ],
+    );
+  } catch (err) {
+    // The partial unique index (project_id, port WHERE state IN active) rejected
+    // this insert because a concurrent start already claimed the slot. That's the
+    // race we deliberately want the DB to arbitrate: just adopt the winning row
+    // instead of creating a duplicate "Port 3000".
+    const isUniqueViolation =
+      err instanceof Error &&
+      /duplicate key value|fs_dev_servers_active_port_uq|unique constraint/i.test(
+        err.message,
+      );
+    if (!isUniqueViolation) throw err;
+
+    const winner = await queryOne<DevServerRow>(
+      `SELECT * FROM fs_dev_servers
+         WHERE project_id = $1 AND port = $2 AND state IN ('starting','running')
+         ORDER BY started_at DESC LIMIT 1`,
+      [opts.projectId, opts.port],
+    );
+    if (winner) return winner;
+    // Extremely unlikely (winner vanished between insert + select): rethrow.
+    throw err;
+  }

  return {
    id,
--- a/vibn-frontend/lib/dev-preview-priority.ts
+++ b/vibn-frontend/lib/dev-preview-priority.ts
@@ -81,7 +81,28 @@ export function sortDevPreviewsFrontendFirst<
    command: string;
    port: number;
    started_at: string | Date;
+    state?: string;
  },
 >(rows: T[]): T[] {
-  return [...rows].sort(compareDevPreviewFrontendFirst);
+  // Defensive dedup: collapse to a single row per port so a stale leftover row
+  // can never render two "Port 3000" entries in the preview dropdown. A running
+  // row always beats a non-running one; ties break on most-recent started_at.
+  const bestByPort = new Map<number, T>();
+  for (const row of rows) {
+    const existing = bestByPort.get(row.port);
+    if (!existing) {
+      bestByPort.set(row.port, row);
+      continue;
+    }
+    const rowRunning = row.state === "running" ? 1 : 0;
+    const existingRunning = existing.state === "running" ? 1 : 0;
+    if (rowRunning !== existingRunning) {
+      if (rowRunning > existingRunning) bestByPort.set(row.port, row);
+      continue;
+    }
+    if (startedAtMs(row.started_at) > startedAtMs(existing.started_at)) {
+      bestByPort.set(row.port, row);
+    }
+  }
+  return [...bestByPort.values()].sort(compareDevPreviewFrontendFirst);
 }