fix(oauth): listen for openExternal spawn errors on darwin and win32

[SebTardif]

A failed macOS open or Windows rundll32 spawn can emit an asynchronous child-process error after openExternal returns. Without an error listener, Node terminates OAuth even though mcporter prints a manual authorization URL. Attach the same listener already used for Linux so the existing manual flow remains usable; browser arguments, shell-free Windows launching, and --no-browser behavior are unchanged.

Thanks @SebTardif for the fix. The maintainer follow-up replaces simulated late-error/global-process handlers with regressions in real Node subprocesses, documents the manual fallback, and adds contributor credit to the changelog.

On Node 24.20.0 / pnpm 10.34.5, pnpm check and pnpm test pass: 1,796 tests passed, 26 skipped. Independent built-runtime proof compares the original baseline ab0c27f03fd7d27aa5b48774ac749a4548836234 with this candidate:

• The Darwin and Windows branches exit 1 with an unhandled ENOENT on base and exit 0 with the fix, using real missing child executables. Linux remains successful in both revisions.
• On native macOS with an empty executable search path, the built OAuth provider crashes on base when open is missing. The fixed provider retains the printed manual URL, accepts a real HTTP request to its localhost callback, and returns the synthetic authorization code. Its callback server is then closed.

Only synthetic data and local processes were used. The Windows branch was exercised on macOS here; cross-platform CI runs the maintained process regressions natively. This does not claim a complete Windows browser/authorization-server session.

Full-candidate isolated Codex review (original patch plus maintainer changes) found no actionable P0–P2 issues. The contributor's original commit is preserved; the maintainer follow-up carries a co-author trailer.

The existing contributor branch was reconciled with main after #343 by a normal merge commit, preserving all contributor history. Frozen install/build, all nine focused OAuth regressions, real missing-executable checks, and the native macOS provider callback proof pass again with the updated dependencies. Full combined P0–P2 reviews before the integration commit and landing found no actionable issues.

Final head: e0ceb538c047e7b0ab2596f8e77a19af1bbbe2b2; base: e212368533bc0bdd94b5a3c89866fd912431d83c. CI passed on Ubuntu, macOS 15, and Windows, including the full suites and real-process regressions. No CI retries were needed.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 4, 2026, 5:04 AM ET / 09:04 UTC.

ClawSweeper review

What this changes

The PR handles asynchronous browser-launch errors on macOS and Windows so OAuth can continue through the printed authorization URL, with regression tests and documentation.

Merge readiness

✅ Ready for maintainer review

Keep open for landing: current main and v0.13.8 still contain the defect. The focused patch has convincing recorded behavior proof and no actionable correctness findings.

Priority: P2
Reviewed head: d598a3b01b8ffb466484a2063192c47b3b8fd076

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	A minimal repair with recorded before/after provider recovery, real-process regressions, and no actionable defects.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The captured body records built-runtime missing-helper failures changing from exit 1 to exit 0, plus native macOS provider recovery through a real localhost callback after open fails. This directly exercises the changed launch owner and continued authorization; native cross-platform CI regressions provide supplemental coverage.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured body records built-runtime missing-helper failures changing from exit 1 to exit 0, plus native macOS provider recovery through a real localhost callback after open fails. This directly exercises the changed launch owner and continued authorization; native cross-platform CI regressions provide supplemental coverage.
Evidence reviewed	10 items	Repository policy and identity: The remote identifies openclaw/mcporter. Root AGENTS.md was read fully; no additional applicable AGENTS.md or maintainer-note files were found. Its preference for focused, verifiable fixes informed this review. Verified introduced change: The complete merge-base-to-head diff contains two production error listeners, 86 test lines, and six documentation/changelog lines. The listeners attach before unref; executable names, arguments, launch options, and Linux handling remain intact. Defect remains on current main: Both macOS and Windows call unref without an error listener; only Linux handles asynchronous launch errors. The surrounding synchronous try/catch cannot catch the later child-process error event.
Findings	None	None.
Security	None	None.

How this fits together

MCPorter's OAuth provider launches a browser for authorization and waits for a local HTTP callback. Browser-launch failures must leave that callback listener available so users can finish authorization manually.

flowchart TD
  A[OAuth authorization request] --> B[Launch system browser]
  B --> C{Browser helper starts?}
  C -->|Yes| D[Browser authorization]
  C -->|No| E[Open printed URL manually]
  E --> D
  D --> F[Local HTTP callback]
  F --> G[Continue OAuth]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +2; tests +86; docs/changelog +6; 0 deletions	The production change is narrowly justified by asynchronous launch failures and accompanied by real-process regression coverage.

Technical review

Best possible solution:

Preserve best-effort browser launching across platforms while keeping the existing manual authorization flow available after helper failures.

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

Yes: current-main source exposes unhandled asynchronous spawn errors when open or rundll32 cannot start, and the supplied before/after runs report that exact failure. This read-only review did not execute the reproduction.

Is this the best way to solve the issue?

Yes: matching Linux's existing error handling is the smallest repair; --no-browser is a deliberate opt-out and does not protect ordinary browser-launch attempts from unexpected failures.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ab0c27f03fd7.

Labels

Label justifications:

• P2: This repairs OAuth termination when a macOS or Windows browser helper cannot start, a bounded failure condition with an existing manual authorization path.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured body records built-runtime missing-helper failures changing from exit 1 to exit 0, plus native macOS provider recovery through a real localhost callback after open fails. This directly exercises the changed launch owner and continued authorization; native cross-platform CI regressions provide supplemental coverage.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured body records built-runtime missing-helper failures changing from exit 1 to exit 0, plus native macOS provider recovery through a real localhost callback after open fails. This directly exercises the changed launch owner and continued authorization; native cross-platform CI regressions provide supplemental coverage.

Evidence

What I checked:

• Repository policy and identity: The remote identifies openclaw/mcporter. Root AGENTS.md was read fully; no additional applicable AGENTS.md or maintainer-note files were found. Its preference for focused, verifiable fixes informed this review. (AGENTS.md:1, d598a3b01b8f)
• Verified introduced change: The complete merge-base-to-head diff contains two production error listeners, 86 test lines, and six documentation/changelog lines. The listeners attach before unref; executable names, arguments, launch options, and Linux handling remain intact. (src/oauth.ts:125, d598a3b01b8f)
• Defect remains on current main: Both macOS and Windows call unref without an error listener; only Linux handles asynchronous launch errors. The surrounding synchronous try/catch cannot catch the later child-process error event. (src/oauth.ts:124, ab0c27f03fd7)
• Latest release also lacks the fix: The v0.13.8 source has the same missing listeners; tag inspection confirms the supplied release commit. This is not an already-shipped fix. (src/oauth.ts:124, 98e739a253fc)
• Existing fallback contract: The provider prints the manual URL after launching the browser and retains its callback server. Existing --no-browser handling bypasses browser launch deliberately; it does not repair unexpected helper failures in normal mode. (src/oauth.ts:508, d598a3b01b8f)
• Recorded real behavior proof: The complete supplied PR body, snapshot sourceRevision c3b8faaacb1df2912e5c304582a3337ad367d783ccfebf4248a55e63f8bb9604, identifies the prepared head and records real missing-executable runs: Darwin/Windows exit 1 on base and 0 after the fix, with Linux successful in both. On native macOS, the built provider survives missing open, preserves the manual URL, accepts a real localhost HTTP callback, returns the synthetic authorization code, and closes the callback server. The Windows branch was exercised on macOS; no complete Windows browser session is claimed. (d598a3b01b8f)

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 (8 earlier review cycles)

• reviewed 2026-09-02T15:32:30.525Z sha ab9afe6 :: needs maintainer review before merge. :: none
• reviewed 2026-09-02T19:24:17.555Z sha ab9afe6 :: needs maintainer review before merge. :: none
• reviewed 2026-09-02T22:50:30.245Z sha ab9afe6 :: needs maintainer review before merge. :: none
• reviewed 2026-09-03T03:59:12.476Z sha ab9afe6 :: needs maintainer review before merge. :: none
• reviewed 2026-09-03T09:29:48.763Z sha ab9afe6 :: needs maintainer review before merge. :: none
• reviewed 2026-09-03T13:07:38.228Z sha ab9afe6 :: needs maintainer review before merge. :: none
• reviewed 2026-09-03T19:53:41.394Z sha ab9afe6 :: needs maintainer review before merge. :: none
• reviewed 2026-09-04T07:01:33.702Z sha ab9afe6 :: needs maintainer review before merge. :: none