Skip to content

Preflight "Playwright not installed" banner is a false positive (bare require in ESM always returns false) #1

Description

@robmclarty-minga

Summary

On any visual (html/tsx) surface, the preflight banner always prints "Playwright not installed" — even when Playwright and the chromium browser binary are correctly installed and the runtime visual sensor works fine. The banner's install check returns false unconditionally because of a bare require in an ESM module.

Version

ridgeline 0.12.6

Symptom

Detected   html/tsx files   →   enabling   Playwright, vision, pa11y, contrast
...
Playwright not installed — visual surface detected; install with: npm install --save-dev playwright && npx playwright install chromium
  Press Enter to continue, Ctrl+C to abort

This fires on every isVisualSurface run regardless of install state.

Root cause

In src/ui/preflight.ts (compiled dist/ui/preflight.js), the banner gate is:

const defaultIsPlaywrightResolvable = () => {
  try {
    require.resolve("playwright");
    return true;
  } catch {
    return false;
  }
};

The package is ESM ("type": "module", and the file uses import). There is no require global in an ESM module, so require.resolve(...) throws ReferenceError: require is not defined, the bare catch swallows it, and the function returns false unconditionally. The banner then fires for every visual-surface run.

Proof

# banner's resolver (bare require in ESM) → always false
node --input-type=module -e "const f=()=>{try{require.resolve('playwright');return true}catch{return false}};console.log(f())"
# → false   (even with playwright installed)

# the pattern the sensor already uses → correct
node --input-type=module -e "import{createRequire}from'node:module';const r=createRequire(import.meta.url);console.log((()=>{try{r.resolve('playwright');return true}catch{return false}})())"
# → true

Scope

Banner-only. The runtime sensor in src/sensors/playwright.js already does this correctly with createRequire(import.meta.url), so the visual gate itself runs fine — the bug is purely the misleading preflight message. (It is, however, a scary message that makes users think their visual checks won't run.)

Suggested fix

Use the same createRequire(import.meta.url) pattern the sensor already uses, e.g. at the top of preflight.ts:

import { createRequire } from "node:module";
const nodeRequire = createRequire(import.meta.url);
// ...
const defaultIsPlaywrightResolvable = () => {
  try {
    nodeRequire.resolve("playwright");
    return true;
  } catch {
    return false;
  }
};

Better: have preflight.ts import and reuse the sensor's existing isPlaywrightResolvable() so there's a single source of truth instead of two copies of the check.

Suggested regression test

A unit test that calls the resolver (or shared helper) in the ESM build with Playwright present and asserts true. The current implementation can only ever return false, so it was never exercised positively.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions