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

[maxmilian]

Fixes #3871

Why

A user reported (Discord #中文区) that clicking Deliver to VS Code does nothing — no handoff and no error. I traced the handoff path and it matches @lefarcen's diagnosis on the issue: POST /api/projects/:id/open-in reports success for a launch the OS actually refused, so the front-end never has anything to surface.

In apps/daemon/src/routes/host-tools.ts the route spawned the editor detached, attached an error listener that swallowed the failure, and then sent { ok: true } synchronously — before the child's error event can even fire (it arrives on a later tick). The web button only shows an error on a non-OK response (openProjectInEditor throws on !resp.ok, HandoffButton renders it via setError + role="alert"), so a refused launch is silently reported as success.

What users will see

When a Deliver-to-editor launch is refused by the OS (e.g. the binary passed the PATH probe but the OS blocks it — quarantine, EACCES), the handoff menu now shows an actionable inline error (Failed to launch <Editor>: <reason>) instead of appearing to do nothing. The success path is unchanged.

No UI was added — this reuses the existing HandoffButton error surface; the fix is daemon-side.

Surface area

• Default behavior change — the /api/projects/:id/open-in endpoint now returns a non-OK error when the OS refuses the launch (previously it always returned { ok: true }). This is the bug fix itself; the success response shape is unchanged and no packages/contracts types changed (the failure reuses the existing API error envelope).
• UI / Keyboard shortcut / CLI / API contract shape / Extension point / i18n / New dependency

Screenshots

N/A — no new UI. The error is rendered by the existing HandoffButton alert surface; only the daemon's failure-reporting changed.

Bug fix verification

• Red-spec test path: apps/daemon/tests/host-tools-routes.test.ts → host tools launch reporting (#3871).
• Did it go red on main and green on the branch? Yes. The fix extracts the spawn into a named helper, launchHostTool, whose invariant is "resolve { ok: false } when the OS refuses the launch." The red spec spawns a non-existent binary and asserts ok === false; the old code path had no failure-reporting (it swallowed error and always returned ok: true), so the spec fails without this change and passes with it. A companion test asserts the success path still reports ok: true.
• Why a helper-level spec rather than an HTTP-boundary e2e: forcing a real spawn failure through the full tools-dev smoke daemon isn't deterministically arrangeable without mocking the OS spawn (an editor would have to pass the PATH probe yet fail to launch). Per AGENTS.md → Bug follow-up workflow ("let the fix read as an invariant" + the cheap-red-spec escape hatch), I encoded the invariant on the extracted helper and kept the route wiring a trivial, type-checked if (!launch.ok) return sendApiError(...).

Validation

• pnpm guard — pass
• pnpm typecheck — pass (clean, all workspace packages)
• Changed-area suite apps/daemon/tests/host-tools-routes.test.ts — pass (4/4).
• Full pnpm --filter @open-design/daemon test — pass (4237 passed). A first run showed one failure in an unrelated file that this PR does not touch; it passed on a clean re-run, i.e. a pre-existing flake, not a regression from this change.

Scope

• Intentionally not changed: editor detection (PR fix(web): make hand-off no-editors fallback perform a real reveal #2494 already handled the zero-editor Finder/Explorer fallback) — this PR is only about reporting launch failures for an editor that is detected.
• Intentionally not added: exit-code inspection. The fix reports the OS-level launch refusal (spawn/error), which is the reported symptom. Surfacing non-zero exit of a detached launcher (e.g. open -a failing after a successful spawn) would need a settle timer and is a separate, riskier change — left as a possible follow-up.
• The route's !launch.ok → 500 EDITOR_LAUNCH_FAILED wiring is a trivial, type-checked mapping and is exercised through the extracted helper rather than a full-server route spec — a route-level launch-failure spec would need global fs/child_process mocks inside the smoke daemon plus a platform-stable editor, which isn't worth the harness fragility here.

Known limitation (Windows)

On win32 the spawn keeps shell: true (required so .cmd/.bat editor shims like code.cmd launch at all). Under cmd.exe a refused/missing command spawns the shell successfully and then exits non-zero rather than emitting an error event, so this fix surfaces launch failures on macOS/Linux but not Windows. A Windows-specific path (e.g. inspecting the shell exit code without hanging on long-lived GUI launchers) is a separate follow-up; the available PATH probe already guards the most common missing-binary case there.

[nettee]

I checked the daemon route change and the helper extraction. The launch/error handling itself looks reasonable, but I left one test-coverage comment because the new spec does not pin the HTTP behavior that actually regressed.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

[nettee]

This new block only exercises launchHostTool directly, so it does not actually lock in the observable regression described in the PR: POST /api/projects/:id/open-in returning a non-OK EDITOR_LAUNCH_FAILED response when the launch is refused. If the route later regressed back to swallowing !launch.ok or mistakenly mapped that branch to 200 { ok: true }, both of these tests would still stay green because the helper invariant would keep passing. Please add one route-level spec that forces the launch path to fail and asserts the HTTP status plus error code/body from /api/projects/:id/open-in, even if that means mocking launchHostTool or child_process.spawn in the daemon test harness.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

[maxmilian]

Added in 8b84778 — host-tools-open-in-route.test.ts now exercises the full route: registerHostToolsRoutes mounted on a real Express app with stub deps, spawn mocked at the node:child_process boundary (per your suggestion) so the refused-launch path runs deterministically on any CI platform, and fs.access mocked so the editor probe resolves without an installed editor.

Assertions lock the observable contract you described: forcing the launch to fail asserts HTTP 500 with error.code === 'EDITOR_LAUNCH_FAILED' and the message body (Failed to launch Cursor: spawn cursor ENOENT), plus the 200 { ok: true, editorId, path } counterpart.

Falsifiability check: re-introducing the swallow (if (false && !launch.ok)) turns the new test red, so a regression back to 200 { ok: true } can't stay green. Daemon typecheck + both host-tools suites pass (6 tests).

[lefarcen]

Hey @maxmilian 👋 — the daemon-side fix is well traced, and the PR body does a great job explaining the async/sync race at the core of the bug. @nettee reviewed and left one actionable comment.

The ask is about locking in the regression at the right boundary: the current spec exercises launchHostTool directly, which is a solid helper-invariant, but it doesn't pin the observable HTTP behavior from POST /api/projects/:id/open-in. If the route wiring later regressed to swallowing !launch.ok, both helper tests would stay green — the actual regression point would go undetected. Adding a route-level spec that asserts the non-OK HTTP status and error body from that endpoint would close that gap. Full context in @nettee's inline comment.

Once that test is in, this should be clean to move forward. 🙌

💡 To drive this PR to merge hands-free, paste this to your AI coding agent (Claude Code / Codex / opencode / Cursor …):
Take over nexu-io/open-design#3952 until it merges — read https://raw.githubusercontent.com/nexu-io/looper/main/skills/pr-takeover/SKILL.md and follow it.

[nettee]

@maxmilian I reviewed the updated daemon launch handling and the added route-level regression coverage. launchHostTool now feeds /api/projects/:id/open-in a real launch-failure path instead of swallowing the async spawn error, and host-tools-open-in-route.test.ts now locks both the 500 EDITOR_LAUNCH_FAILED response and the success response at the HTTP boundary. Nice follow-through on closing the earlier test-gap cleanly.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

[lefarcen]

@maxmilian nice execution on the route-level spec — mocking spawn at the node:child_process boundary and the falsifiability re-introduction check are exactly right. @nettee's approved the updated coverage. ✅

CI is fully green on the new head. The PR is currently gated on an admin-side step for external PRs; the maintainer team will clear it to merge — nothing more needed on your side.