Skip to content

feat(react): /react-doctor umbrella skill + in-house browser core + debug job#853

Open
aidenybai wants to merge 39 commits into
mainfrom
feat/react-skill
Open

feat(react): /react-doctor umbrella skill + in-house browser core + debug job#853
aidenybai wants to merge 39 commits into
mainfrom
feat/react-skill

Conversation

@aidenybai

@aidenybai aidenybai commented Jun 17, 2026

Copy link
Copy Markdown
Member

Based on main. The diff is the /react-doctor skill umbrella + the in-house browser core + the debug job. The React DevTools profiler harness lives inside @react-doctor/browser (no separate perf-agent package).

Grows React Doctor from a code scanner into one skill, /react-doctor, that helps the agent write better React, scans its own changes in the background, and can drive a real browser to profile performance, reproduce bugs, and review UI.

Usage lives in the skill, not this description. skills/react-doctor/SKILL.md is the source of truth for how the agent routes work and drives the browser, with the per-job playbooks in skills/react-doctor/references/{performance,debug,design,explain}.md. This PR describes what changed; read the skill for how to use it. The only slash command is /react-doctor (force a job with /react-doctor perf); there is no /doctor.

What's here

  • /react-doctor umbrella skill — the scanner-only skill grows into one skill that routes across jobs: ten always-on React baseline rules plus prose routing across doctor (static scan + 0–100 score, runs in the background), perf (React render + V8 CPU profiling), debug (reproduce in a real browser), design (measured UI + motion review), and rule explain/configure. The command, package name, and brand stay React Doctor; the browser jobs only run when asked.
  • @react-doctor/browser — a thin playwright-core wrapper that attaches to a running Chrome over CDP (logins/cookies come along), launching its own persistent, detached Chrome only as a fallback so the page survives across CLI invocations. playwright-core is an optional dependency loaded lazily, so Playwright never bundles into the CLI hot path. Also houses the React DevTools profiler harness in src/react-profiler.
  • react-doctor browser commandsopen, eval, snapshot, screenshot, all sharing --cdp, --no-launch, and a one-shot --viewport WxH emulation. The model is one verb to drive the page (eval, with the Playwright page in scope) and one flag to measure it (--profile): --profile returns the whole runtime picture in one pass (console, network, LoAF/LCP/CLS jank ranked by input-blocking time, an axe-core a11y audit, a React render profile, and a V8 CPU profile). eval --codegen writes a runnable Playwright .spec.ts from a verified action; eval --video [path] records a .webm screencast (Playwright 1.59+ / ffmpeg). Both flags are mirrored on the browser_eval MCP tool. The full command reference and recipes live in the skill.
  • react-doctor debug + @react-doctor/debug — a local NDJSON logging server (react-doctor debug serve, the default subcommand) the debug job posts runtime evidence to. Lock reuse, dedup, and daemon/--json modes so the server outlives the spawning CLI process and the agent just gets the endpoint back.
  • react-doctor mcp + @react-doctor/mcp — a Model Context Protocol server over stdio (the bin fast-paths mcp) exposing doctor_scan, the browser_* tools, and the debug_* log server, so any MCP-capable agent can call the jobs directly. Debug binds are loopback-only with a minted-endpoint allowlist.
  • Playbooksreferences/{debug,design,performance,explain}.md, followed per job.

Hardening (from dogfooding eval --profile)

Profiling react-grab on a live 40k-element app surfaced three issues in the tool itself, now fixed:

  • Launch fallback survives a busy debug port. When 9222 is held by another app, the fallback no longer spawns Chrome onto the occupied port and fails; it launches on a free port and persists the endpoint so later commands reattach to that same instance (new is-port-available / find-available-port / read|write-launched-endpoint utils).
  • LoAF ranks by jank, not length. The performance report ranked frames by total duration, surfacing multi-second non-blocking frames (idle/first-paint artifacts) over real jank. It now drops non-blocking frames and ranks by input-blocking time.
  • The CPU profile no longer profiles itself. The React DevTools export serialized in-page while V8 sampling was still running, polluting the "hottest functions" list with our own overhead. The CPU profiler and timeline trace now stop before the export.

Build / checks

  • build, typecheck, lint, format green for @react-doctor/core, @react-doctor/browser, @react-doctor/debug, and react-doctor.
  • @react-doctor/browser, @react-doctor/debug, install-react-doctor, install-agent-hooks, parse-viewport, and strip-unknown-cli-flags suites pass; full react-doctor suite green.
  • The skill ships to dist/skills/react-doctor.
  • Merged up to current main; repo-wide typecheck and lint are green.

Deliberately deferred (follow-ups)

  • Ambient task-end static-scan hook (on by default, conservative threshold)
  • Publishing story for @react-doctor/browser and @react-doctor/debug (workspace deps today)
  • product-thinking pass for the browser / debug CLI surface

Note

Medium Risk
Large new surface area (browser CDP, MCP tools, debug HTTP server) with security-sensitive paths mitigated by loopback-only MCP debug binds and minted-endpoint allowlists; optional Playwright/ffmpeg and Chrome install remain user-environment variables.

Overview
Expands React Doctor from a static scanner into a /react-doctor umbrella: the in-repo agent skill moves to a symlink under skills/react-doctor, and react-doctor install gains --global (home agent dirs) while install scripts and preview builds can pin npx to a REACT_DOCTOR_PACKAGE_SPECIFIER (e.g. pkg.pr.new).

Adds three internal packages wired into the CLI: @react-doctor/browser (CDP attach/launch, Playwright eval/inspect, React DevTools profiling, axe, CPU/timeline analysis), @react-doctor/debug (NDJSON log server with per-project lock reuse), and @react-doctor/mcp (stdio MCP with doctor_scan, browser_*, debug_*). The bin shim fast-paths mcp like experimental-lsp; playwright-core is optional/lazy.

react-doctor browser exposes open, eval, snapshot, screenshot, close with shared CDP/viewport flags. eval is the main driver: void actions return the a11y tree; --profile records console/network/perf/memory/geometry/a11y/React/CPU and writes a DevTools trace; --codegen emits a Playwright spec; --video records .webm (Playwright 1.59+). Profile runs keep data when the action fails (evalError); page errors surface in eval output.

react-doctor debug serve starts the logging server (daemon/--json for agents). CI now packs the CLI once on Linux and smoke-tests the tarball on Windows/macOS; pkg.pr.new publishes deslop-js too and bakes the preview URL into builds.

Reviewed by Cursor Bugbot for commit 135fa5d. Bugbot is set up for automated code reviews on this repo. Configure here.

@pkg-pr-new

pkg-pr-new Bot commented Jun 17, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/deslop-js@853
npm i https://pkg.pr.new/eslint-plugin-react-doctor@853
npm i https://pkg.pr.new/oxlint-plugin-react-doctor@853
npm i https://pkg.pr.new/react-doctor@853

commit: 135fa5d

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

React Doctor skipped this pull request — it changed no React files.

Reviewed by React Doctor for commit 135fa5d.

cursor[bot]

This comment was marked as resolved.

@aidenybai aidenybai changed the base branch from main to cursor/perf-agent-devtools-setup-d63d June 17, 2026 20:49
@aidenybai aidenybai changed the title feat: /react-doctor skill umbrella + in-house browser core (debug/design/grab) feat(react): /react umbrella skill + in-house browser core + debug-agent debug job Jun 17, 2026
cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

Comment thread packages/browser/src/connect.ts Outdated

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread packages/browser/src/perf-observer.ts Outdated
Comment thread packages/browser/src/session.ts Outdated
Comment thread packages/react-doctor/src/cli/commands/browser.ts Outdated
@aidenybai

Copy link
Copy Markdown
Member Author

/rde parity

@react-doctor-evals

react-doctor-evals Bot commented Jun 18, 2026

Copy link
Copy Markdown

❌ Parity failed — trace d4ed45083ac35e225cd9a55783f6c52b. Check server logs for that trace ID.

@aidenybai

Copy link
Copy Markdown
Member Author

/rde parity

Comment thread packages/browser/src/perf-observer.ts
Comment thread packages/react-doctor/src/cli/commands/debug.ts
@aidenybai aidenybai changed the title feat(react): /react umbrella skill + in-house browser core + debug-agent debug job feat(react): /react-doctor umbrella skill + in-house browser core + debug-agent debug job Jun 18, 2026
Comment thread packages/debug/src/server.ts Outdated
Comment thread packages/react-doctor/src/cli/commands/debug.ts
Comment thread packages/browser/src/session.ts Outdated
@aidenybai aidenybai changed the base branch from cursor/perf-agent-devtools-setup-d63d to main June 19, 2026 03:03
Comment thread packages/debug/src/server.ts Fixed
Comment thread packages/debug/src/server.ts Fixed
Comment thread packages/browser/src/session.ts
…bot)

A persisted launched-Chrome endpoint (often a non-default port) was tried
first on every attach. Once that instance died, each command paid the full
5s CDP timeout against the dead port before falling back to the default. Now
when the fallback to the default Chrome succeeds, the dead launched endpoint
is cleared, so later commands skip it instead of timing out every time.
Comment thread packages/browser/src/session.ts
…undles

The published react-doctor CLI bundles the private @react-doctor/browser
package, but the bundler only inlined it implicitly. When it was reached
transitively (dist/mcp.js → @react-doctor/mcp → @react-doctor/browser) the
bundler could externalize it instead, emitting a phantom import of a private
package the published tarball never declares (caught by check:published-deps,
issue #629). Declare it in each pack's alwaysBundle so it is always inlined or
the build fails loudly, never silently externalized.
Comment thread packages/browser/src/session.ts
…ils (bugbot)

Three gaps in the new self-reporting eval, all surfacing precisely when an
action breaks — when the diagnostics matter most:

- evaluateOrSnapshot dropped the collected page errors when the expression
  threw (the "Errors during eval" section was skipped on the throw path). It
  now appends them to the thrown message so the real cause rides along.
- inspect (eval --profile / browser_eval profile) threw the whole recording
  away when the action failed. It now captures the failure as `evalError` and
  still returns the console/network/CPU/timeline/React picture as its context.
- console/network capture started before settle() while the profilers started
  after, so they mixed pre-action load traffic into the window. Capture now
  starts after settle, covering the same window as the profilers.
Comment thread packages/browser/src/perf-observer.ts
… (bugbot)

The first profile after a load replayed buffered long-animation-frame and
layout-shift entries from initial page load, because the per-page watermark
started at -1 — so LoAF rows and CLS mixed load jank into a window that console,
network, CPU, and timeline only cover post-action.

Replace the persisted watermark with a per-call `sinceMs` recording-start
timestamp captured right before the driven action: buffered observers now skip
every entry at or below it, which both excludes pre-action load jank and dedupes
frames an earlier no-reload run already counted — the same guarantee the
watermark gave, minus the global page state.
Comment thread packages/browser/src/session.ts Outdated
… (record .webm)

Two artifacts off the same verified `browser eval` run, since the expression
already runs as Playwright code with `page` in Node scope:

- --codegen drives the expression, then writes a runnable Playwright spec that
  navigates to the session's current URL, replays the action, and asserts no
  console/page errors fired (the signal eval already reports) — a verified
  interaction becomes a regression test in one step. A failing action throws
  instead of writing a green-looking test.
- --video records a .webm of the page while the expression runs, for playback,
  in any mode (plain / --profile / --codegen). Uses Playwright's imperative
  screencast (1.59+) — the only video API that records a CDP-attached page; a
  missing ffmpeg surfaces an actionable `npx playwright install ffmpeg` hint.

Both are mirrored on the browser_eval MCP tool (codegen, video args). Extracts
isEvalExpression from compile-eval for reuse; bumps playwright-core to ^1.59.0.
Comment thread packages/react-doctor/package.json
…loor (bugbot)

- inspect captured recordingStartMs (the LoAF/CLS floor) only after the React
  start + scroll read, so LoAF/CLS omitted the arming gap the timeline/CPU trace
  included, despite the comment claiming one shared window. Capture it the instant
  the recorders are armed, so the setup work falls inside every signal's window.
- react-doctor still declared optional playwright-core ^1.49.1 while the browser
  package needs ^1.59.0 for page.screencast (--video). Raise the floor so an
  install can't resolve a Playwright without the screencast API.
Comment thread packages/browser/src/perf-observer.ts
…ed smoke)

@react-doctor/debug is private (never published) and statically imported by the
CLI, so it must inline into dist/cli.js like @react-doctor/browser already does.
It was relying on default bundling, which one platform emitted as a phantom
external import instead — resolving locally but breaking `npm i react-doctor`,
which the packed-CLI smoke caught on Windows. Declare it in alwaysBundle so the
build inlines it deterministically (and fails loudly if it ever can't).
Comment thread packages/browser/src/session.ts
A beta tester installing a pkg.pr.new preview previously got a skill and an
`install` step that both pointed at the published `react-doctor@latest`, so the
agent never actually exercised the previewed branch. Route every install-time
package reference (the dev-dep `install` adds, the `doctor` package script, the
manual-install hint) through one build-injected PACKAGE_SPECIFIER, and rewrite
the shipped skill's `npx` commands to match at build time.

The publish-any-commit workflow stamps each preview build with its own immutable
pkg.pr.new URL (REACT_DOCTOR_PACKAGE_SPECIFIER), declared on turbo's build env so
the per-commit value busts the cache. Released builds leave it unset, so the
specifier defaults to react-doctor@latest and the skill ships verbatim — zero
behavior change for published releases.

No changeset: the published package's behavior is unchanged by default; this is a
distribution mechanism plus a behavior-preserving refactor of the install code.
Comment thread packages/browser/src/session.ts Outdated
The recording-start floor for LoAF/LCP/CLS was an absolute performance.now()
captured in Node before the driven action. Two gaps:

- A navigating action (page.goto) resets the new document's performance timeline
  to ~0, so every new-document entry sorted below the pre-nav floor and got
  dropped — --profile runs that navigate reported no blocking frames and zero
  CLS for the loaded page.
- LCP was never floored at all, so a no-reload run could replay the initial
  navigation's LCP.

Stash the recording-start timestamp on the page itself (a window marker set when
the recorders arm) and read it in-page. A navigation wipes the marker with the
old document, so the new page reads 0 and keeps its full load vitals (the
navigation is the measured event); a no-reload run still floors out pre-action
jank. Apply the same floor to the LCP observer.
Comment thread packages/browser/src/connect.ts
The packed-cli smoke built and packed its own tarball on each OS, so the Windows
leg tested a Windows-built bundle that never ships — rollup there intermittently
externalizes the private @react-doctor/debug workspace dep, and turbo's per-OS
cache made the failure stick across reruns. npm releases and pkg.pr.new previews
both build on Linux, so the published CLI bundle is always the Linux one.

Pack the publishable tarballs once on the Linux test leg, upload them as an
artifact, and add a smoke-packed-cli-cross-os job that installs that exact
artifact on Windows and macOS. The CLI bundle is platform-independent JS, so the
only per-OS variables left are install, native-binding resolution, and runtime —
which is what the cross-OS smoke should actually exercise. The smoke script gains
--pack-only / --tarballs modes; with neither flag it still packs+verifies in one
temp dir for local runs.
…n Linux

check:published-deps and the packed-cli pack both consume turbo's shared,
content-hashed build cache. A single runner whose rollup externalizes the
private @react-doctor/debug workspace dep (instead of inlining it) was uploading
that broken react-doctor bundle under the shared hash, and every OS — including
the Linux leg that builds the published release and the pkg.pr.new preview — then
restored it via FULL TURBO and failed (debug as a phantom external import).

Fold RUNNER_OS into the build task hash (mirroring how the test task folds in
MATRIX_NODE_VERSION) so each OS keeps its own build cache and a divergent build
on one can never cross-contaminate the artifact another ships. Also add
@react-doctor/debug to the MCP pack entry's alwaysBundle for parity with the CLI
entry, matching the comment that already claims it's inlined.
…ched one (bugbot)

When a recorded launched instance (on a non-default port) was briefly unreachable
and another Chromium app squatted on 9222, reattach fell back to that foreign
browser, cleared the saved launched endpoint, and ran later commands against the
wrong session — orphaning our launched profile.

Only fall back to the well-known default when launching is disabled (attaching to
whatever is there is then the only option) or on a cold start (the user's own
Chrome). With launching enabled we relaunch our own instance instead, so a slow
or mid-restart launched Chrome is never silently swapped for an unrelated one.
react-doctor depends on `deslop-js: workspace:*`, but the pkg-pr-new publish set
omitted it, so the preview tarball shipped a raw `workspace:*` spec for it.
`npx https://pkg.pr.new/react-doctor@<sha> install` then failed with
EUNSUPPORTEDPROTOCOL. pkg-pr-new only rewrites a workspace dep to its preview URL
when the package is in the publish command, so add ./packages/deslop-js.
Comment out the "Select additional React Doctor setup" prompt and the
pre-commit / agent-hook install steps so `install` no longer prompts for
or installs them. Their now-unused helpers, constants, imports, and the
hook-installation tests are commented out alongside, keeping lint/typecheck
clean. Flip the flags + uncomment the blocks to restore.
The skill installs as `/react-doctor`; `/doctor` was never a real slash
command. Point both the triage trigger (SKILL.md) and the explain
playbook cross-reference at `/react-doctor`.
Exclude first-party agent-install from the pnpm minimumReleaseAge guard so
the latest release installs without the vetting delay.
Comment thread packages/browser/src/session.ts Outdated
Add a `--global` flag and an interactive "Where should the skill be
installed?" prompt to `react-doctor install`, threading agent-install's
`global` option so the skill can land in each agent's home dir
(~/.cursor, ~/.claude, …) for every project. Defaults to project-local;
non-interactive runs stay local unless `--global` is passed. Records the
scope on the install.completed wide event.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5b4136c. Configure here.

Comment thread packages/browser/src/session.ts
Fold the recording-start marker write into the awaited React-profiler
evaluate so a failed write can't be silently swallowed and leave a stale
floor; a no-reload re-run then can't over-count buffered LoAF/CLS from an
earlier run. A page that can't take the marker now fails the inspect
loudly (matching the React-profiler start it rode beside). (bugbot)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants