fix(daemon): surface detached launch failures to callers

[steipete]

Supersedes #347, keeping @SebTardif's original commit and sign-off at the base of this branch.

Problem

launchDaemonDetached spawned the daemon with { detached: true, stdio: 'ignore' } and called unref() without an error listener. Node emits spawn failures (ENOENT and friends) asynchronously, so a missing launch command — stripped PATH, missing nohup on a compiled macOS binary, a deleted Node entry — became an unhandled 'error' event that killed the parent CLI. This is the same defect class as #341 for openExternal.

What changed beyond the original fix

@SebTardif's commit attached the listener, which stopped the crash but swallowed the cause: callers then sat through the full readiness timeout with no indication of what failed.

launchDaemonDetached now awaits process creation and resolves on 'spawn' or rejects on 'error', wrapping the failure with the executable and the original OS error as cause. Both call sites — mcporter daemon start and DaemonClient.ensureDaemon() auto-launch — await it, so a launch failure is reported immediately instead of masquerading as a readiness timeout. The 'error' listener stays attached after a successful spawn so a late error still cannot become an unhandled event.

Authenticated readiness, its 45-second timeout, the foreground logging guidance, and the 0.13.9 single-user ownership safeguards are unchanged. Other daemon and relay spawn sites were checked and already handle errors.

Verification

pnpm check clean. pnpm test: 1867 passed, 26 existing skips, 218 files. New tests cover real ENOENT failures through both callers, spawn ordering, and repeated late errors. No timeouts, CI budgets, or skips were changed.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 4:24 PM ET / 20:24 UTC.

ClawSweeper review

What this changes

Make explicit and automatic daemon starts report process-creation failures immediately, with the executable, original OS error, regression coverage, and documentation.

Merge readiness

✅ Ready for maintainer review

This PR remains useful: main prevents the crash but still discards the launch error. No actionable correctness findings remain, and the verified test merge resolves the previous conflict concern.

Priority: P2
Reviewed head: e3292d899b221067023b79cb8b52c56457f9f24e

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused implementation with appropriate regression coverage and no blocking findings.
Proof confidence	🌊 off-meta tidepool	Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate and does not materially change authority; real-process regressions cover the launch helper and both callers.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate and does not materially change authority; real-process regressions cover the launch helper and both callers.
Evidence reviewed	6 items	Repository policy: Read the complete root AGENTS.md and VISION.md. Only the root AGENTS.md was found; no .agents maintainer-notes directory exists. Applied the guidance favoring focused fixes and regression coverage. Tests were not executed because this review prohibits builds and generated artifacts. Main still lacks immediate error propagation: Inspected main’s launch helper: its error listener discards the failure and returns void. The supplied GitHub context confirms #347 merged that crash-prevention change; it does not implement this PR’s caller-visible rejection. Both production callers preserve readiness checks: The helper resolves on spawn, rejects with the original cause on error, and retains its error listener. Both production call sites await it before their existing authenticated readiness checks; automatic-launch cleanup still clears startingPromise in finally.
Findings	None	None.
Security	None	None.

How this fits together

MCPorter’s daemon keeps MCP server connections alive across CLI invocations. Explicit starts and automatic launches create the background process, then verify authenticated readiness before using it.

flowchart TD
  A[Explicit daemon start] --> C[Detached process launch]
  B[Automatic daemon launch] --> C
  C --> D{Process created?}
  D -->|No| E[Return executable and OS error]
  D -->|Yes| F[Authenticated readiness check]
  F --> G[Ready daemon or diagnostic timeout]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +19/-10; tests +95/-26	The small production increase implements error propagation, with regression coverage for both callers and child-process event ordering.

Technical review

Best possible solution:

Preserve immediate process-creation diagnostics while retaining authenticated readiness and its existing timeout for successfully spawned processes.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: a missing launch executable on main has its error discarded, after which callers enter readiness polling. The PR adds real-process regression cases; this read-only review did not execute them.

Is this the best way to solve the issue?

Yes. Awaiting process creation in the shared helper and both callers is a narrow repair that preserves successful-launch readiness and existing security checks.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 7a2d4dc563c9.

Labels

Label justifications:

• P2: This is a bounded daemon-startup diagnostic fix for process-creation failures.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate and does not materially change authority; real-process regressions cover the launch helper and both callers.

Evidence

What I checked:

• Repository policy: Read the complete root AGENTS.md and VISION.md. Only the root AGENTS.md was found; no .agents maintainer-notes directory exists. Applied the guidance favoring focused fixes and regression coverage. Tests were not executed because this review prohibits builds and generated artifacts. (AGENTS.md:1, e3292d899b22)
• Main still lacks immediate error propagation: Inspected main’s launch helper: its error listener discards the failure and returns void. The supplied GitHub context confirms fix(daemon): handle spawn errors on detached launch #347 merged that crash-prevention change; it does not implement this PR’s caller-visible rejection. (src/daemon/launch.ts:37, 7a2d4dc563c9)
• Both production callers preserve readiness checks: The helper resolves on spawn, rejects with the original cause on error, and retains its error listener. Both production call sites await it before their existing authenticated readiness checks; automatic-launch cleanup still clears startingPromise in finally. (src/daemon/launch.ts:29, e3292d899b22)
• Regression coverage and reported validation: Read both changed test files. Real Node subprocess cases exercise the launch helper and both production callers with a missing executable; unit cases cover spawn ordering, synchronous errors, EACCES, and repeated late errors. The captured PR body reports pnpm check and 1,867 passing tests with 26 existing skips. These are contributor-reported results, not reviewer executions. (tests/daemon-launch-process.integration.test.ts:61, e3292d899b22)
• Previous conflict blocker resolved: Raw test-merge parents are exactly pinned main followed by the reviewed head. Its production diff contains only the intended launch changes. Supplied checks show successful Ubuntu and macOS builds, with Windows still running; routine check completion does not establish a new review defect. (b79a0bd280b7)
• Area history and inspection limits: Main history connects Sebastien Tardif to the merged launch-error handling and Peter Steinberger to the shared-daemon refactor and earlier launch maintenance. Bounded blame has boundary markers and is not introduction proof. Older-head and release-blob inspection failed because unavailable objects could not be fetched; live GitHub checks also failed on network resolution. The pinned source and supplied context remain sufficient for this keep-open verdict. (src/daemon/launch.ts, 7a2d4dc563c9)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• SebTardif: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-05T20:11:21.995Z sha 63f288c :: blocked before merge. :: none