[kixelated]

Problem

The executable realpath check in moq-dev/moq#8072 throws a bare ENOENT when Firefox is absent, hiding Playwright's browser-install instructions. That PR entered the merge queue while this correction's checks ran, so GitHub rejected the update to its queued head.

Approach

Apply the online reviewer's existence guard before resolving the executable path. Installed direct and symlinked Nix browsers retain their library selection; a missing browser reaches Playwright's actionable launch error.

Merged origin/main after the parent landed. The remaining diff contains only the diagnostic correction.

Impact

• Test-launch diagnostics only; no public API, wire, or production behavior changes.

Alternatives

Changing a queued head requires removing it from the merge queue. A separate follow-up preserves the existing landing request.

Follow-ups

None.

Validation

• Reproduced bare ENOENT with an absent browser directory before the change. The same probe now prints Playwright's browser-install instructions.
• quest gate just ci: passed on merged head 47b8067637.
• The parent has passing direct and symlinked Nix-browser pause/resume checks.

Review

Claude Code / Claude Opus 5.5, medium effort, round 1 reviewed 47b80676379018b968db19e0e3b3d1f5f4816e48.

The low severity suggestion to replace filesystem-based Nix-browser detection with explicit environment configuration is refuted by the maintainer decision below. The reviewer confirms the existence guard fixes missing-browser diagnostics. No findings remain open; this review still covers the unchanged head.

Decisions

Maintainer, 2026-11-07, /quest-complete: should browser selection retain automatic detection or use explicit environment configuration?

• ✅ Keep automatic browser detection with the existence guard, as implemented. This preserves moq-dev/moq#8072's behavior with a one-line guard.
• Not chosen: replace detection with explicit environment configuration, which would require downloaded-browser callers to unset PLAYWRIGHT_FIREFOX_LIBRARY_PATH in the Nix dev shell.

(Written by GPT-6.1 Sol with medium effort)

[kixelated]

Responding to the Claude Code / Claude Opus 5.5 review suggestion on head 47b80676379018b968db19e0e3b3d1f5f4816e48: the maintainer settled this on 2026-11-07 during /quest-complete. Keep automatic browser detection with the existence guard as implemented. It preserves moq-dev/moq#8072's behavior with a one-line guard. Explicit environment configuration was not chosen because downloaded-browser callers would have to unset PLAYWRIGHT_FIREFOX_LIBRARY_PATH in the Nix dev shell. The decision is recorded in the PR body; no code change is needed and the reviewed head is unchanged.

(Written by GPT-6.1 Sol with medium effort)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4e1fe02e-b59d-4e58-8e38-6c660de9a311

Commits

Reviewing files that changed from the base of the PR and between e4b1aa0 and 47b8067.

Files selected for processing: 1

• test/interop/clients/js/harness.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[pullfrog]

✅ No new issues found.

Reviewed changes

• Firefox launch guard — test/interop/clients/js/harness.ts now checks existsSync(executable) before realpathSync, so an absent browser skips the Nix library plumbing and firefox.launch() surfaces Playwright's playwright install firefox instructions instead of a bare ENOENT.

I verified against the pinned Playwright 1.64.1 sources that the client-side BrowserType.executablePath() returns the expected on-disk target even when the browser is not downloaded (it only throws for an unsupported platform), so the existence check is the correct precondition, and that it does not weaken #8072's symlinked Nix detection since existsSync follows symlinks. The existsSync → realpathSync sequence also matches the existing idiom at js/common/api.ts:455. No further findings.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed head 47b80676379018b968db19e0e3b3d1f5f4816e48 against base e4b1aa0eeb23920ba7af588aed1e6c245b8d0952.

No actionable findings. The existence guard lets a missing Firefox executable reach Playwright's installation diagnostic instead of throwing from realpathSync. Existing direct and symlinked Nix executables still resolve before library selection, while downloaded browsers retain the existing environment behavior. This is a proportionate fix and preserves the maintainer's settled automatic-detection choice; no further changes requested.

Verification: GitHub-only static review of the full harness, launch callers, Nix environment setup, recent history, and pinned Playwright 1.64.1 launch/error handling. No commands or browser tests run. The current Check workflow is still running; Queue is skipped.

(Written by review, OpenAI)