fix(daemon): report Deliver-to-editor launch failures instead of swallowing them

apps/daemon/src/routes/host-tools.ts

@@ -218,6 +218,39 @@ export async function resolveHostToolLaunchPlan(
   };
 }
 
+// Spawn a detached host-tool launch and wait for the OS to confirm it
+// actually started. Node emits `spawn` once the child is running and `error`
+// when the launch is refused (missing binary, quarantine, EACCES). The
+// `error` event arrives on a later tick, so the route must await this before
+// replying — otherwise it reports success for a launch the OS rejected and
+// the user sees nothing happen (#3871).
+export function launchHostTool(
+  command: string,
+  args: string[],
+): Promise<{ ok: true } | { ok: false; error: string }> {
+  return new Promise((resolve) => {
+    // Detached so the daemon doesn't keep the child alive; same shape paseo
+    // uses (CLI shim, `open -a`, Explorer, xdg-open, etc.).
+    const child = spawn(command, args, {
+      detached: true,
+      stdio: 'ignore',
+      shell: process.platform === 'win32',
+    });
+    let settled = false;
+    child.once('spawn', () => {
+      if (settled) return;
+      settled = true;
+      child.unref();
+      resolve({ ok: true });
+    });
+    child.once('error', (err) => {
+      if (settled) return;
+      settled = true;
+      resolve({ ok: false, error: err instanceof Error ? err.message : String(err) });
+    });
+  });
+}
+
 function applicableForPlatform(entry: CatalogueEntry, platform: Platform): boolean {
   if (platform === 'unknown') return false;
   if (entry.platforms && !entry.platforms.includes(platform)) return false;
@@ -306,21 +339,20 @@ export function registerHostToolsRoutes(app: Express, ctx: RegisterHostToolsRout
       if (!launchPlan.available || !launchPlan.command || !launchPlan.args) {
         return sendApiError(res, 409, 'EDITOR_NOT_AVAILABLE', `${entry.label} is not installed`);
       }
-      // Detached spawn so the daemon doesn't keep the child alive; same
-      // shape paseo uses. Each catalogue entry turns the project dir into
-      // the native argument shape it expects (CLI shim, `open -a`, Explorer,
-      // xdg-open, etc.).
-      const child = spawn(launchPlan.command, launchPlan.args, {
-        detached: true,
-        stdio: 'ignore',
-        shell: process.platform === 'win32',
-      });
-      child.on('error', () => {
-        // Swallow — best-effort; the client will see ok:true but the OS
-        // might still have refused (e.g. quarantine). Real diagnostic
-        // path is `od project open-in --debug`.
-      });
-      child.unref();
+      // Wait for the OS to confirm the launch before replying. Previously
+      // this returned ok:true synchronously and swallowed the child's `error`
+      // event, so a refused launch was reported as success and the user saw
+      // nothing happen — the web button only surfaces an error on a non-OK
+      // response (#3871).
+      const launch = await launchHostTool(launchPlan.command, launchPlan.args);
+      if (!launch.ok) {
+        return sendApiError(
+          res,
+          500,
+          'EDITOR_LAUNCH_FAILED',
+          `Failed to launch ${entry.label}: ${launch.error}`,
+        );
+      }
       const body: OpenProjectInEditorResponse = {
         ok: true,
         editorId,

apps/daemon/tests/host-tools-open-in-route.test.ts

@@ -0,0 +1,111 @@
+// Route-level coverage for POST /api/projects/:id/open-in (#3871).
+//
+// The helper-level tests in host-tools-routes.test.ts pin launchHostTool's
+// contract, but not the route's translation of a refused launch into an HTTP
+// response — if the route regressed back to swallowing `!launch.ok` (or mapped
+// it to `200 { ok: true }`), those tests would stay green. Here the spawn is
+// mocked at the node:child_process boundary so the full route path runs, and
+// the assertions lock the observable behavior: HTTP status + error code/body.
+
+import { EventEmitter } from 'node:events';
+import type http from 'node:http';
+import type { AddressInfo } from 'node:net';
+import path from 'node:path';
+import { tmpdir } from 'node:os';
+import express from 'express';
+import type { Response } from 'express';
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
+
+import { registerHostToolsRoutes } from '../src/routes/host-tools.js';
+import type { RegisterHostToolsRoutesDeps } from '../src/routes/host-tools.js';
+
+const spawnState = vi.hoisted(() => ({ fail: false, error: 'spawn cursor ENOENT' }));
+
+// Deterministic spawn: emits `error` or `spawn` on the next tick depending on
+// spawnState, so both launch outcomes are reachable on any CI platform.
+vi.mock('node:child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:child_process')>();
+  return {
+    ...actual,
+    spawn: vi.fn(() => {
+      const child = new EventEmitter() as EventEmitter & { unref: () => void };
+      child.unref = () => {};
+      setImmediate(() => {
+        if (spawnState.fail) child.emit('error', new Error(spawnState.error));
+        else child.emit('spawn');
+      });
+      return child;
+    }),
+  };
+});
+
+// Make the $PATH probe succeed everywhere so resolveHostToolLaunchPlan
+// reports the editor as available and the route reaches the launch step.
+vi.mock('node:fs/promises', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs/promises')>();
+  return { ...actual, access: async () => undefined };
+});
+
+// Absolute baseDir short-circuits projectHostOpenDir, so resolveProjectDir is
+// never consulted and no real project layout is needed.
+const PROJECT_DIR = path.join(tmpdir(), 'od-3871-project');
+
+let server: http.Server;
+let baseUrl: string;
+
+beforeAll(async () => {
+  const app = express();
+  app.use(express.json());
+  registerHostToolsRoutes(app, {
+    db: {},
+    http: {
+      // Mirrors the compat shape of server.ts sendApiError: status + { error: { code, message } }.
+      sendApiError: (res: Response, status: number, code: string, message: string) =>
+        res.status(status).json({ error: { code, message } }),
+    },
+    paths: { PROJECTS_DIR: tmpdir() },
+    projectStore: {
+      getProject: (_db: unknown, id: string) =>
+        id === 'p1' ? { id, metadata: { baseDir: PROJECT_DIR } } : null,
+    },
+    projectFiles: { resolveProjectDir: () => PROJECT_DIR },
+  } as unknown as RegisterHostToolsRoutesDeps);
+  server = app.listen(0);
+  await new Promise<void>((resolve) => server.once('listening', () => resolve()));
+  baseUrl = `http://127.0.0.1:${(server.address() as AddressInfo).port}`;
+});
+
+afterAll(async () => {
+  await new Promise<void>((resolve) => server.close(() => resolve()));
+});
+
+function postOpenIn(projectId: string) {
+  return fetch(`${baseUrl}/api/projects/${projectId}/open-in`, {
+    method: 'POST',
+    headers: { 'content-type': 'application/json' },
+    body: JSON.stringify({ editorId: 'cursor' }),
+  });
+}
+
+describe('POST /api/projects/:id/open-in launch reporting (#3871)', () => {
+  it('returns 500 EDITOR_LAUNCH_FAILED when the OS refuses the launch', async () => {
+    spawnState.fail = true;
+
+    const resp = await postOpenIn('p1');
+
+    expect(resp.status).toBe(500);
+    const body = (await resp.json()) as { error: { code: string; message: string } };
+    expect(body.error.code).toBe('EDITOR_LAUNCH_FAILED');
+    expect(body.error.message).toContain('Failed to launch Cursor');
+    expect(body.error.message).toContain('spawn cursor ENOENT');
+  });
+
+  it('returns 200 ok:true once the OS confirms the launch', async () => {
+    spawnState.fail = false;
+
+    const resp = await postOpenIn('p1');
+
+    expect(resp.status).toBe(200);
+    expect(await resp.json()).toEqual({ ok: true, editorId: 'cursor', path: PROJECT_DIR });
+  });
+});

apps/daemon/tests/host-tools-routes.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from 'vitest';
 
-import { resolveHostToolLaunchPlan } from '../src/routes/host-tools.js';
+import { launchHostTool, resolveHostToolLaunchPlan } from '../src/routes/host-tools.js';
 
 describe('host tools open-in launch plans', () => {
   it('uses the absolute macOS open command to reveal project folders in Finder', async () => {
@@ -23,3 +23,24 @@ describe('host tools open-in launch plans', () => {
     expect(plan.args).toEqual(['-a', 'Terminal', '/tmp/open-design-project']);
   });
 });
+
+describe('host tools launch reporting (#3871)', () => {
+  it('reports ok once the OS confirms the process spawned', async () => {
+    // process.execPath (the running node binary) always spawns, so this
+    // exercises the success path without depending on an installed editor.
+    const result = await launchHostTool(process.execPath, ['--version']);
+
+    expect(result.ok).toBe(true);
+  });
+
+  it('surfaces the launch failure instead of swallowing it', async () => {
+    // shell:true on win32 runs the command through cmd.exe, which exits
+    // non-zero rather than emitting an `error` event for a missing binary.
+    if (process.platform === 'win32') return;
+
+    const result = await launchHostTool('open-design-nonexistent-editor-3871', []);
+
+    expect(result.ok).toBe(false);
+    if (!result.ok) expect(result.error).toBeTruthy();
+  });
+});