* perf: reduce cold scan startup and add V8 profiling
Add repeatable CPU and heap profiling so cold-run bottlenecks can be measured, then remove profile-proven startup, traversal, and security-scan overhead.
* perf: add deterministic stress benchmark
Exercise cold scans with reproducible diagnostics and remove redundant visitor-map allocations so regressions are measurable and behavior stays hash-verifiable.
* fix(tests): normalize benchmark paths cross-platform
Resolve the absolute fixture path through Node so the harness expectation matches Windows path semantics.
* perf: avoid effect analysis parent traversal
Reuse Oxc visitor keys with a parent-safe fallback so effect scope analysis no longer strips and restores every AST parent reference.
* perf: reduce repeated semantic traversal
Reuse host visitors and shared analysis caches to avoid redundant cold-scan AST passes while preserving diagnostic parity.
* fix: harden performance regression coverage
* fix(perf): support profiling on Node 20
* fix(plugin): skip CFGs for bodyless functions
* fix: harden profiling and semantic compatibility
Prevent benchmark artifacts and host differences from corrupting comparisons, while preserving host AST and React Compiler compatibility across optimized scan paths.
* fix(plugin): preserve rule correctness in fast scan paths
Keep security prefilters comment-tolerant and resolve React HOC wrappers by binding provenance so scan optimizations cannot hide valid diagnostics.
* fix: address review findings and consolidate duplicated code
- wrapWithSemanticContext copies the rule's visitors instead of mutating
a possibly shared object; walkAst regains its null-root guard
- no-multi-comp HOC identity accepts React-compat runtimes via
REACT_RUNTIME_MODULE_SOURCES (now incl. @wordpress/element) across
ESM, require(), and TS import-equals, with regression coverage in
both directions
- shared traversal core (forEachChildNode) replaces the walkChildren and
containsJsx copies; isImportedFromReact deduped into is-react-api-call
- performance harness consolidated (27 -> 22 files): shared commander
options, shared profile-frame accumulation, record shape guards, dead
BenchmarkSample.profileDirectory removed
- CLI-spawning harness tests skip without a built dist; dead and
tautological test assertions removed
Net -282 LOC against the branch head.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(plugin): capture the Program root for every rule
Rules can reach context.scopes through shared helpers and factories
(createDeprecatedReactImportRule resolves namespace aliases via
resolveConstIdentifierAlias), so the codegen'd requiresSemanticContext
flag — a regex over the rule's own file — silently left factory-based
rules on stub scope analyses: no-react-dom-deprecated-apis and
no-react19-deprecated-apis stopped reporting namespace usages through
the host wrapper.
Delete the flag and its regex detector entirely and install the
root-capture Program visitor on every rule. The analyses stay lazy and
memoized per Program, so rules that never read them still pay only one
call per rule per file, and no future helper-routed consumer can be
missed. Regression test runs the real host-wrapped rule and fails
against the gated wrapper.
Fixes cursor bugbot review finding on PR #1114.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(scripts): reject cyclic heap-profile node graphs during collection
collectNodes flattened the parsed tree with no visited guard, so a
cyclic or shared-node object graph would loop before reaching the
duplicate-ID check. Unreachable through analyzeHeapProfiles (JSON.parse
output is always a strict tree), but guard and throw like the CPU
analyzer so a synthetic graph fails deterministically.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Rayhan Noufal Arayilakath <me@rayhanadev.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: exclude TypeScript 6.x to fix bunx installation crash
TypeScript 6.0.3 has an internal circular dependency with its Comparison
enum that triggers a known Bun module loader bug, causing
`bunx react-doctor install` to crash with "ReferenceError: Cannot access
'Comparison' before initialization".
Narrow the dependency range from `>=5.0.4 <7` to `>=5.0.4 <6` until Bun
fixes enum initialization order (see oven-sh/bun#12805).
`npx` continues to work because npm's resolver handles the circular
dependency correctly. TypeScript 5.9.3 is stable and tested; TypeScript
6.x support will return once the upstream bug is resolved.
Closes#962
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
* fix(deslop-js): pin typescript <6, drop no-op core pin
The bunx install crash is react-doctor's own startup `import typescript` (already
pinned <6); the residual vector is the dead-code scan loading deslop-js, which
still declared `typescript@^6.0.3`. Pin deslop-js to `>=5.0.4 <6` too — 487
deslop-js tests pass under 5.9.3. Revert the @react-doctor/core hunk: core is
private and never published, so narrowing its range reached no consumer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Co-authored-by: Rayhan Noufal Arayilakath <me@rayhanadev.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(core): add concrete-semver parsing and comparison util
Adds parseConcreteSemver/compareConcreteSemver/formatConcreteSemver for
exact major.minor.patch versions, rejecting ambiguous range specs so a
security check never guesses a resolved version. Exposes PackageJson.version
for reading installed manifests under node_modules.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* feat(core): flag React Server Components security advisories by version
Adds a no-vulnerable-react-server-components environment check that detects
projects running RSC on a version with a known advisory: the critical
unauthenticated RCE (CVE-2025-55182, CVSS 10.0) and the later high-severity
DoS (CVE-2026-23870).
- Cross-framework path (Vite, Parcel, React Router, Waku, RedwoodSDK):
resolves the installed react-server-dom-{webpack,parcel,turbopack} version
and compares against the per-minor patched releases (19.0.6/19.1.7/19.2.6).
- Next.js special plumbing: Next vendors its own RSC runtime, so a Next
project is checked by its next version (15.x->15.5.18, 16.x->16.2.6, 13/14
unsupported) and the fix points at a Next.js bump, not a React bump.
- Stays quiet for pure client-side React apps and never flags off an
ambiguous declared range whose lockfile may resolve to a patched version.
Wired into the run-inspect environment phase alongside checkPnpmHardening.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* refactor(core): address review on RSC advisory check
- Use the `semver` library instead of a hand-rolled version parser/comparator
(removes packages/core/src/utils/concrete-semver.ts and its test).
- Resolve installed versions across every workspace package's node_modules in
addition to the scan root, so a monorepo whose `next`/`react-server-dom-*`
is installed only under e.g. apps/web is still detected (fixes Bugbot
'Misses workspace node_modules installs').
- Add committed test fixtures under
tests/fixtures/check-react-server-components-advisory and drive the
version-matrix suite off them; keep programmatic tests for node_modules
resolution, the monorepo regression, and canary handling.
- Changeset bumped to patch; drop the redundant PackageJson.version comment.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* fix(core): dispatch RSC advisory on resolved next, not root framework
A monorepo root classified as Vite (or unknown) with Next.js installed only
in a workspace previously skipped the Next.js advisory check, missing the
vendored RSC runtime. Dispatch now keys on whether `next` resolves in any
candidate directory. Fixes Bugbot 'Non-Next framework skips Next check'.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* fix(core): check every workspace install for RSC advisories
Replaces single first-match resolution with a per-directory scan so:
- An inert/safe Next at the monorepo root no longer hides a vulnerable Next
in a workspace (Bugbot 'First next wins over workspace').
- A pre-13 Next install no longer suppresses the standalone react-server-dom
check, since pre-13 is not an affected line (Bugbot 'Pre-13 next skips RSC
check').
Diagnostics are deduplicated by message across directories.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* fix(core): walk to true monorepo root for RSC advisory candidates
When scanning a nested workspace package, deps may be hoisted to the real
monorepo root. Walk up via findMonorepoRoot so that root (and its
node_modules) and every sibling workspace are probed, instead of only the
scanned directory. Fixes Bugbot 'Workspace scan misses monorepo root'.
Fixture tests are copied into an isolated temp dir so the walk-up does not
reach the react-doctor repo root they live under.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* fix(core): don't let an unparseable catalog spec shadow resolved next version
resolveVersionInDirectory now falls through to the first spec that is a
concrete version, so a `catalog:` (or other unparseable) manifest spec no
longer shadows discovery's already-resolved `project.nextjsVersion` pin.
Fixes Bugbot 'Catalog spec blocks version override'.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* fix(core): probe all workspaces, not just React-bearing ones
Enumerate every workspace package directory for candidate node_modules
probing instead of reusing the React-filtered listWorkspacePackages, so a
workspace declaring only react-server-dom-* (or next under optionalDependencies)
is still evaluated. Fixes Bugbot 'Skips non-React workspace packages'.
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Nisarg Patel <NisargIO@users.noreply.github.com>
* feat(cli): add Sentry crash reporting
Capture uncaught CLI errors via @sentry/node, flushed before the process
exits and enriched with a run-context snapshot (version, node/platform/
arch, invocation command/argv, cwd, CI provider, coding agent,
interactivity, JSON mode). Initialized as the first statement of the CLI
entry and scoped to the CLI only — the @react-doctor/api library never
initializes Sentry. Opt out via --no-score (also skipped under tests).
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(cli): consolidate CI/agent detection into is-ci-environment
build-run-context re-declared CI provider and coding-agent env-var maps
that mirrored is-ci-environment. Move the label maps and the
detectCiProvider/detectCodingAgent resolvers into is-ci-environment as the
single source of truth, derive the flat agent env-var list from the brand
map, and have isCodingAgentEnvironment derive from detectCodingAgent.
Behavior is unchanged (env-detection tests still green).
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
The React Compiler rules (`react-hooks-js/*`) were silently missing in
published builds because `resolveReactHooksJsPlugin` resolved
`eslint-plugin-react-hooks` via `createRequire(import.meta.url)` — which
found the package in the monorepo's devDependencies but not in the
npm-installed package where it was only an optional peer dep.
Move `eslint-plugin-react-hooks` from optional peerDependencies +
devDependencies to a real dependency in both `@react-doctor/core` and
`react-doctor` so the compiler rules fire automatically for any project
with `babel-plugin-react-compiler` detected.
* refactor: collapse @react-doctor/types and @react-doctor/project-info into core
Eliminates two private workspace packages whose only role was to
namespace shared TS types and project-discovery helpers — neither
was a real consumer boundary, and the indirection cost a `workspace:*`
edge per leaf package without saving any code.
**Layout**
- `packages/types/` → `packages/core/src/types/` (10 files, pure TS types)
- `packages/project-info/` → `packages/core/src/project-info/`
(27 files, discoverProject + framework detection + monorepo helpers)
- `packages/types/src/react-native-dependency-names.ts` →
`packages/oxlint-plugin-react-doctor/src/react-native-dependency-names.ts`
(the canonical RN-aware-package list lives next to its heaviest
consumer — the rule gate — and is re-exported from core to avoid
the rule-plugin ↔ core dependency cycle the previous types package
was specifically designed to break).
The two packages are deleted from the workspace; every consumer
(`react-doctor`, `api`, `oxlint-plugin-react-doctor`, `actions/review`,
+130 source/test files) now imports from `@react-doctor/core` directly.
**Errors cleanup**
Dropped the legacy plain-class `ReactDoctorError` and `isReactDoctorError`
from `project-info`. The four narrow errors (`ProjectNotFoundError`,
`NoReactDependencyError`, `PackageJsonNotFoundError`,
`AmbiguousProjectError`) now extend `Error` directly — the only valid
hierarchy is the tagged `Schema.TaggedErrorClass` `ReactDoctorError`
from `core/errors.ts`, which has been the canonical one since the
Effect v4 refactor. Added `isProjectDiscoveryError` as a narrow type
guard for the four leaves.
The discovery errors are thrown synchronously BEFORE the Effect
runtime takes over (at `diagnose()` / CLI entry points); inside the
runtime, the Project service's `translateProjectInfoError` converts
each one to its tagged-reason equivalent.
**Schema vs type name collisions**
`schemas.ts` exports `Diagnostic` / `JsonReport` / `JsonReportMode` /
… as Schema.Class values, while `types/` exports the same names as
TS types. The core barrel re-exports the TS-type versions
(broad-strokes API) and exposes the Schema classes via a new
`@react-doctor/core/schemas` subpath export for the two callers
that need them (`Reporter` service, smoke validator, schema tests).
**Validation**
- `pnpm typecheck` ✓
- `pnpm lint` ✓
- `pnpm format` ✓
- `pnpm test` ✓ (1449 passing, 3 skipped — no regressions)
- `pnpm smoke:json-report` ✓
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(core): keep RN-dependency constants out of types barrel to avoid pulling oxlint plugin into discovery
Bugbot caught (medium severity): re-exporting `isReactNativeDependencyName` from
`oxlint-plugin-react-doctor` via the core types barrel meant every consumer of
`@react-doctor/core` (including `discoverProject` / `discoverReactSubprojects`)
would load the entire 286-rule plugin at module-init.
Adds a tiny standalone leaf at `core/src/project-info/internal-rn-dependency-names.ts`
(zero imports, just the constants the workspace-discovery gate needs) and switches
`is-package-json-react-native-aware.ts` to use it. The types barrel no longer
re-exports the plugin-side constants — rule consumers import them directly from
`oxlint-plugin-react-doctor`.
The duplication is intentional and documented at both files; the two lists are
short enough that a future regression test in the plugin can observe the union.
Validated: typecheck + test (1449 pass) + lint + format all green.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(action): move legacy composite action to actions/inspect/
The new root action.yml will host the PR Review pipeline.
The existing 'scan + sticky comment' composite action now lives at
actions/inspect/action.yml so consumers can opt into either action
explicitly:
- millionco/react-doctor@v1 -> PR Review (new)
- millionco/react-doctor/actions/inspect@v1 -> Legacy scan flow
Updates the github-action.test.ts contract test to read from the
new path.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* feat(action): add React Doctor Review composite action
Self-hosted GitHub Action that mirrors the hosted react.review bot's
doctor-only review pipeline. No hosted service required — everything
runs in-process on the GitHub Actions runner.
What the action does on every pull_request (opened/synchronize/reopened):
- Reads the event payload from GITHUB_EVENT_PATH and extracts the
owner/repo/PR number/head SHA/base ref. Detects fork PRs via head.repo
vs base.repo full_name mismatch.
- Resolves the merge base via octokit.repos.compareCommitsWithBasehead,
using ${headOwner}:${headRef} as the head identifier for fork PRs.
- Materializes the base commit alongside the runner's checkout of head
via 'git fetch --depth=1 <remote> <baseSha>' + 'git worktree add
--detach <baseDir> <baseSha>'. The base remote URL embeds
https://x-access-token:<token>@github.com/... so private repos work.
- Lists changed files via octokit.pulls.listFiles (paginated) and
derives addedLineContents: Map<lineNumber, content> per file from the
unified-diff patch — required for inline-comment line resolution.
- Runs react-doctor's diagnose() API in-process on both the head
checkout and the base worktree. Pre-discovers React subprojects via
workspaces in package.json + pnpm-workspace.yaml (using
discoverReactSubprojects from @react-doctor/project-info), fans out
per project, combines diagnostics with paths remapped to be relative
to the repo root, and weight-averages scores by source file count.
- Diffs head diagnostics against base diagnostics using a
(relativePath, rule, message) key with count deltas (not boolean
presence) so duplicate diagnostics are counted correctly.
- Inline comment candidates are net-new diagnostics with
severity === 'error' whose line landed on a + line in the diff.
- Reconciles existing inline threads via GraphQL
repository.pullRequest.reviewThreads. Threads whose body contains the
INLINE_COMMENT_MARKER and whose threadKey matches a current candidate
are skipped. Threads whose key is no longer in the candidate set get
PATCHed with '✅ Addressed in <headSha>' and resolved via the
resolveReviewThread GraphQL mutation.
- Posts remaining candidates via octokit.pulls.createReview({ event:
'COMMENT' }), capped at MAX_INLINE_COMMENTS_COUNT (25) per run.
- Upserts the sticky summary comment identified by the
STICKY_COMMENT_MARKER HTML marker. Posts a pending-review comment
before analysis starts, swaps it for the final regression/no-issues
body once diagnostics are computed, and falls back to an analysis-
failure body in the catch path.
- Creates a check run on the head SHA before analysis, then completes
it with the assessment body (score line, new-diagnostics list,
project metadata table).
Edge cases handled:
- Fork PRs: warns when head.repo differs from base.repo and gracefully
catches write failures on inline / sticky posts when the workflow
token lacks write access. Consumers can pass a PAT or App
installation token via the 'token' input to enable fork-PR review.
- NoReactDependencyError / PackageJsonNotFoundError /
ProjectNotFoundError: concludes the check run as 'skipped' with a
'not a React project' summary and deletes the pending comment
instead of loud-failing.
- Diagnostic path resolution tries: absolute under root, absolute
under project root, relative + project-prefixed, and 'already
prefixed' in that order.
- Cleans up the base worktree via 'git worktree remove --force' in a
finally block.
Token strategy (dual-mode):
- Default: workflow GITHUB_TOKEN (action.yml default). Comments author
as github-actions[bot]. Same-repo PRs work out of the box; fork PRs
get warnings and best-effort posts.
- Override: pass a PAT or App installation token via the 'token' input
for fork-PR support or branded identity.
Not in scope (intentionally out):
- No LLM / agent layer — doctor-only review.
- No Vercel Sandbox dependency — diagnose() runs in-process.
- No Upstash rate limit / no-react cache — single-repo CI doesn't need
multi-tenant guardrails.
- No OIDC token-broker mode — that flow lives in the hosted react.review
service; this action is standalone.
File layout:
- action.yml at the repo root (composite, runs tsx against the
entrypoint). Installs only the @react-doctor/review-action and
react-doctor workspace closures via pnpm --filter to keep cold-start
install footprint minimal.
- actions/review/ is a new workspace package
(@react-doctor/review-action) with three TypeScript modules:
- src/pipeline.ts — pure helpers (diff parser, diagnose-across-
workspace, computeDiagnosticsDelta, formatters, inline-candidate
builder, check-run assessment).
- src/github.ts — Octokit IO (compareCommitsWithBasehead, listFiles,
sticky upsert/delete, inline review, GraphQL thread reconciliation,
check-run create/complete).
- src/index.ts — entrypoint orchestration.
- src/constants.ts holds magic numbers (MAX_INLINE_COMMENTS_COUNT,
sticky/inline markers, check-run name, GraphQL page size).
Dogfood:
- .github/workflows/doctor-action.yml runs the action against this
repo's own PRs via 'uses: ./'.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* docs(readme): document Review and inspect actions
Adds a 'PR Review (recommended)' section showing the new root action
and keeps the previous scan + sticky comment flow under
'Scan + sticky comment (legacy)' pointing at
millionco/react-doctor/actions/inspect@main.
Existing references to 'millionco/react-doctor@main' for the legacy
inputs (directory/verbose/project/diff/github-token/fail-on/offline/
annotations/node-version) are repointed to the new
millionco/react-doctor/actions/inspect@main path so existing recipes
keep working without breaking on the new Review action's inputs.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(action): disambiguate workspace filter to skip website build
Both the workspace root and packages/react-doctor are named 'react-doctor'.
'pnpm --filter react-doctor build' matched the root, whose build script
is 'turbo run build' — which fans out to every workspace, including
website. website#build then failed in CI because the filtered install
(by design) did not pull in Next.js.
Switch both pnpm invocations to path-style filters / direct cwd:
- Install: --filter './packages/react-doctor...' (instead of bare name).
- Build: cd packages/react-doctor && pnpm build (run the package's
vp-pack script directly, bypassing the name collision).
Verified locally:
cd packages/react-doctor && rm -rf dist && pnpm build # OK
pnpm install --frozen-lockfile \
--filter '@react-doctor/review-action...' \
--filter './packages/react-doctor...' # Scope: 6 of 9
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(action): build workspace deps via turbo so dist/ resolves at runtime
The previous fix ran 'pnpm build' directly in packages/react-doctor,
which only bundled that one package. vp pack treats workspace deps as
external imports (Could not resolve '@react-doctor/core' ... treating
it as an external dependency), so the resulting dist/index.js still
'import { ... } from "@react-doctor/core"' at runtime. The runtime
import resolved to packages/core/dist/index.js — which was never
built — and the action crashed:
Error [ERR_MODULE_NOT_FOUND]: Cannot find module
'.../packages/core/dist/index.js' imported from
'.../packages/react-doctor/dist/index.js'
Use 'turbo run build --filter=./packages/react-doctor' instead. The
path filter avoids the workspace name collision (root + packages/
react-doctor share the name 'react-doctor'), and turbo's
'dependsOn: ["^build"]' in turbo.json triggers a topological build
of the four upstream workspace deps (@react-doctor/types, /project-
info, /core, oxlint-plugin-react-doctor) before react-doctor itself.
Verified locally: 'rm -rf packages/*/dist && pnpm exec turbo run build
--filter=./packages/react-doctor' produces dist/ in all five packages.
Turbo is a root devDependency and gets installed even with the
filtered install scope, so no install-step change required.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* style: drop em-dashes from new action source and README additions
Replace em-dashes (—) with plain ASCII alternatives (':', '(' ')',
or '-') in the new action.yml, actions/review/src/index.ts,
actions/review/src/pipeline.ts, and the two README lines added for
the PR Review / inspect-action documentation.
Pre-existing em-dashes elsewhere in the README are not touched —
they live in sections this PR does not modify.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(action): drop unsafe +++/--- skips from diff parser
Bugbot caught a medium-severity bug in parseAddedLineContents: the
'rawLine.startsWith("+++ ")' / 'rawLine.startsWith("--- ")' guards
were copy-pasted from full unified-diff parsing but are wrong for
GitHub's pulls.listFiles patches.
GitHub's patch field begins at the first '@@ ...' hunk header and
never includes the '+++ b/file' / '--- a/file' file headers. So the
guards do not fire on any legitimate header, but they DO fire on
legitimate added/removed lines whose payload begins with '++ ' or
'-- ' (e.g. C++ '++ i', '++ x = 1', shell '-- arg'). When they fire
on a '+' line, currentNewLine is not incremented, off-by-oneing every
subsequent line number in the hunk and silently mis-placing inline
review comments.
Removed both guards; the '@@' hunk parser is the only thing that
should set/reset currentNewLine. Also removed the unused '\\' (no
newline at end of file marker) branch since it is a no-op and not
emitted by GitHub patches anyway.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(action): move @react-doctor/project-info to dependencies
Bugbot flagged that pipeline.ts imports 'discoverReactSubprojects'
from '@react-doctor/project-info' at runtime, but the manifest listed
the package under devDependencies. It worked in CI only because the
filtered install does not pass --prod and so dev deps get installed
too. Move it to dependencies so a future --prod install does not
silently break the action.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* refactor(action): dedupe isMissingReactProjectError into pipeline.ts
Bugbot flagged that isMissingReactProjectError was identically defined
in both pipeline.ts and index.ts with the same imports from
react-doctor/api. Export it once from pipeline.ts and import in
index.ts so the two copies cannot diverge. Also lets index.ts drop
the NoReactDependencyError / PackageJsonNotFoundError /
ProjectNotFoundError imports from react-doctor/api since the helper
encapsulates the instanceof checks.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(action): diff diagnostics at (line, column) granularity
Bugbot caught a medium-severity bug in computeDiagnosticsDelta. The
prior implementation grouped by (relativePath, rule, message) and
sliced 'headOccurrences.slice(baseCount)' to pick the 'new'
occurrences. Linters emit diagnostics in line order, so slicing the
tail systematically picks the highest-line occurrences as new. When
a genuinely new violation appeared at a *lower* line than an
existing one with the same key, the old (higher-line) occurrence was
labelled new instead, then buildInlineCommentCandidates rejected it
(its line was not on a '+' line in the diff) and the inline comment
was silently dropped.
Diff at the (line, column) sub-key. For each (relativePath, rule,
message) key we now build a multiset of base positions, walk the
head occurrences, and only flag head positions not present in base
as new. Same path for fixed (base positions not present in head).
Line-shifted occurrences naturally fall out as one 'fixed' + one
'new' but the +/- line filter in buildInlineCommentCandidates
discards the shifted-context noise.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* chore(action): drop unused SCORE_GAUGE_DOMAIN / GITHUB_ACTIONS_BOT_LOGIN
Bugbot flagged these as dead exports — they were not referenced from
anywhere in the codebase. Removing them per the AGENTS.md 'Remove
unused code' rule.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(action): tolerate spaces in thread-key file paths
Bugbot flagged that THREAD_KEY_HEADER_PATTERN matched the key as
'([^\s]+)' but buildThreadKey concatenates the raw relativePath. A
file path containing a space (say, 'src/My Component.tsx') would
yield a thread key like 'src/My Component.tsx:5|rule-name' that the
regex truncated to 'src/My' at extraction time. The truncated key
never matches any candidate so the next run silently resolves the
thread as 'addressed' instead of leaving it active.
Loosen to '([^\n]+?)' (non-greedy, single-line). Equivalent for the
99% case (paths without whitespace) and correct for the path-with-
spaces case.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* refactor(action): single inline marker for ownership + thread key
Bugbot flagged duplicated marker logic: formatInlineCommentBody
prepended a plain <!-- react-doctor-review-inline --> line, then
buildInlineCommentBodyWithKey prepended a separate keyed
<!-- react-doctor-review-inline:KEY --> line, and isOwnedReviewThread
relied on the plain marker. If the plain prefix ever drifted, thread
reconciliation would silently fail because owned threads would no
longer match.
Collapse to one marker source. INLINE_COMMENT_MARKER becomes
INLINE_COMMENT_MARKER_PREFIX (with the trailing ':'); the keyed body
builder builds the marker from that constant; ownership detection
substring-matches on the same prefix; the threadKey regex extracts
everything between the ':' and ' -->'. formatInlineCommentBody no
longer needs to seed the body with a marker because the keyed
prefix is always prepended at post time.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(action): null head repo + don't conflate AmbiguousProjectError with skip (bugbot)
Two Bugbot findings on the rebased PR:
### 1. Null head repo crashes context
\`pullRequest.head.repo\` is nullable (deleted forks, restricted
fork metadata). Earlier code did
\`pullRequest.head.repo?.full_name.split("/")[1]\` — \`?.\` only
guards \`repo\`, so \`full_name\` would be undefined and \`.split\`
crashes before the review runs. The same null case also set
\`headRepoFullName\` to the base repo name, so \`isFork\` became
\`false\` and fork-specific compare / permission behavior was wrong.
Funnel every field through a single \`headRepoMeta\` guard, and
make \`isFork\` only true when \`head.repo\` is present AND its
\`full_name\` differs from the base.
### 2. ReactDoctorError conflated with skip
\`runDiagnoseAcrossWorkspace\` and the orchestrator's catch block
were collapsing any \`isReactDoctorError(error)\` to "not a React
project" / "skip this workspace entry". \`AmbiguousProjectError\`
is also a \`ReactDoctorError\` but means "multiple React roots
were found here" — silently treating it as "skip" hides a real
misconfiguration and contradicts the PR's stated handling for
\`NoReactDependencyError\` / \`PackageJsonNotFoundError\` /
\`ProjectNotFoundError\` only.
Dropped \`|| isReactDoctorError(error)\` from both catch blocks so
only \`isMissingReactProjectError\` suppresses; ambiguous /
invalid-config / scan-failure ReactDoctorError instances now
propagate as hard errors. Cleaned up the now-unused
\`isReactDoctorError\` import in \`index.ts\` and \`pipeline.ts\`.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(action): scope base worktree scan to same INPUT_DIRECTORY as head (bugbot)
Bugbot caught: when the action runs against a non-\`.\` \`directory\`
input (the monorepo / per-package scoping case), head analysis
correctly scoped to that subdirectory (\`resolveHeadDirectory\`
resolved \`workspace\` + \`INPUT_DIRECTORY\`) but base analysis ran
against the full worktree root. Diagnostic paths and counts then
disagreed across the two snapshots, so regressions, sticky
summaries, and inline comments on added lines could be wrong or
missing.
Extracted \`resolveInputDirectory()\` to centralize the
\`INPUT_DIRECTORY || "."\` lookup, and added
\`resolveBaseScanDirectory(worktreeDirectory)\` that joins
\`INPUT_DIRECTORY\` onto the worktree the same way
\`resolveHeadDirectory\` joins it onto \`GITHUB_WORKSPACE\`. The
base \`runDiagnoseAcrossWorkspace\` call now sees the same subtree
as the head call.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(action): anchor diagnostic paths to workspace root so PR matching works under INPUT_DIRECTORY (bugbot)
The previous fix corrected the scan directory for the base
worktree but exposed a path-key mismatch: \`pulls.listFiles\`
returns changed-file keys relative to the **repository root**
(e.g. \`packages/my-app/src/App.tsx\`), but \`runDiagnoseAcrossWorkspace\`
was emitting \`relativePath\`s relative to the SCAN root (e.g.
\`src/App.tsx\` when the scan was scoped to \`packages/my-app\`).
\`buildInlineCommentCandidates\`'s \`changedFilesByPath.get(diagnostic.relativePath)\`
then always missed for monorepo / scoped-directory configs, so
no inline comments got posted and \`reconcileInlineThreads\` saw
an empty active set — silently resolving real regressions while
the sticky summary still listed them.
\`runDiagnoseAcrossWorkspace(rootDirectory, pathBaseDirectory?)\`
now accepts a second arg that overrides the directory the output
\`relativePath\`s are computed against. Defaults to \`rootDirectory\`
for the standalone-CLI case; the action passes
\`resolveWorkspaceRoot()\` for HEAD and the worktree root for BASE,
so emitted paths line up with the GitHub API's repo-root keys
regardless of \`INPUT_DIRECTORY\` scoping.
Renamed \`resolveHeadDirectory\` → uses new \`resolveWorkspaceRoot()\`
helper for DRY.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(action): keep threads alive for regressions that can't be posted inline (bugbot)
\`reconcileInlineThreads\` was being passed only the threadKeys that
PASSED the inline-comment filter (errors with non-null \`patch\` on
an added line). A real regression that survives the head-vs-base
diff but can't be inline-posted — e.g. \`pulls.listFiles\` returned
a null \`patch\` (binary / rename-only / large file), the violation
line landed on a context line within the hunk instead of a \`+\`
line, or the file was modified outside the diff window — would be
omitted from \`activeThreadKeys\`. The existing review thread then
got the "Addressed" footer and resolved on the next push, even
though the regression is unchanged and the sticky summary still
lists it.
Build \`activeThreadKeysFromRegressions\` from every error-severity
entry in \`newDiagnostics\` (using the same \`buildThreadKey\` shape
as \`buildInlineCommentCandidates\`), then union with the
inline-candidate keys. The set passed to \`reconcileInlineThreads\`
now reflects "still a regression" rather than "still inline-postable",
so threads stay alive while the underlying violation persists.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(action): scan PR head SHA, not merge commit; prune stale worktrees (bugbot)
Two bugbot findings:
### 1. Scans merge commit not head (high severity)
\`actions/checkout@v5\` on a \`pull_request\` event checks out the
synthetic merge commit (\`refs/pull/N/merge\`) by default, not the
PR head. So workspace HEAD is the merge result while inline
reviews + the check run annotate \`pull_request.head.sha\` —
diagnostic line numbers can disagree with what GitHub renders
in the PR diff.
Fixes:
- README example workflow + repo fixture workflow now pass
\`ref: \${{ github.event.pull_request.head.sha }}\` to
\`actions/checkout\` so HEAD matches the SHA we annotate.
- Action source verifies workspace HEAD == \`pull_request.head.sha\`
at startup and logs a loud \`::warning::\` with the exact
\`ref: ...\` snippet to add when they disagree. The action
still runs (so existing workflows don't break overnight),
but the user sees the actionable diagnostic in the log.
### 2. Stale worktree breaks reruns (medium severity)
\`materializeBaseWorktree\` was \`fs.rmSync\`-ing the worktree
directory before \`git worktree add\`. If a prior run left the
worktree registered in \`.git/worktrees/\` (interrupted job,
cleanup failed), \`worktree add\` rejects with "already registered"
even though the directory is gone.
Added \`tryRunGit(["worktree", "prune"], headDirectory)\` between
the \`rmSync\` and the \`add\` — drops stale registrations
defensively. \`tryRunGit\` because a fresh repo has nothing to
prune and we don't want to abort on success.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(action): include message-hash in thread key so same-rule same-line distinct messages stay distinct (bugbot)
\`buildThreadKey\` was \`(relativePath, line, rule)\` but the
regression-diffing identity in \`computeDiagnosticsDelta\` is
\`(relativePath, rule, message)\`. So two net-new errors with the
same rule firing on one line but different messages (e.g. a
multi-arg rule that complains about each invalid prop separately)
would collapse to a single thread key — only one inline comment
gets posted, and reconciliation can't tell the second violation
ever existed when it gets fixed.
Added a compact FNV-1a 32-bit hash of the message (base36-encoded
so the key stays short, ~7 chars) and appended it to the thread
key as a fourth segment. The hash is deterministic across runs
so reconciliation continues to match the same thread on later
pushes; multiple distinct messages on the same \`(path, line, rule)\`
now keep their own threads.
Updated both call sites (\`buildInlineCommentCandidates\` in
\`pipeline.ts\` + the active-threadKeys union in \`index.ts\`) to
pass \`diagnostic.message\` through to \`buildThreadKey\`.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(action): dedupe inline candidates by threadKey + document line-shift limitation (bugbot)
Two related bugbot findings:
### 1. Duplicate inline keys in review (fixed)
\`buildInlineCommentCandidates\` could emit multiple candidates
with the same \`threadKey\` (the same \`(path, line, rule, message)\`
identity firing twice in one scan — e.g. a rule that double-fires
on identical source). \`createReview\` would either duplicate-post
the comment or reject the whole batch.
Switched the candidate collector from an array push to a
\`Map<threadKey, candidate>\` keyed by the resolved thread key.
First-seen wins; subsequent duplicates are dropped before posting.
### 2. Line shift resolves threads (documented limitation)
When a push moves a surviving violation to a different line, the
old thread key no longer matches and the previous thread gets
the "Addressed" footer + resolution even though the regression
persists. This is a pre-existing limitation of the
\`(path, line, rule, message)\` thread-key design — removing
\`line\` would conflate distinct violations of the same rule
across multiple lines, so the fix requires tracking original-line
anchors per thread (server-side state), which is out of scope
for an in-repo action.
Added a long-form \`KNOWN LIMITATION\` comment on \`buildThreadKey\`
documenting the tradeoff and what a proper fix would need so
future readers don't re-discover the issue from scratch.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Drops the last imperative `node:child_process.spawnSync` call out
of the Git service path. The `Git.layerNode` service now spawns
through Effect v4's native subprocess primitives:
- `ChildProcess.make("git", args, { cwd, extendEnv: true })`
from `effect/unstable/process` (builds the Command AST)
- `ChildProcessSpawner` from the same module (the spawning
service tag, provided by `@effect/platform-node`'s
`NodeServices.layer`)
- `Stream.decodeText` + `Stream.mkString` to drain stdout/stderr
concurrently with `handle.exitCode` via `Effect.all({
concurrency: 3 })`, so the pipe never blocks on a full buffer
- `Effect.scoped` to bind the child process to a Scope so
cancellation / interruption cleans up the subprocess
- `Effect.catchTag("PlatformError", ...)` to fold the platform
error into the existing tagged `ReactDoctorError({ reason:
GitInvocationFailed })` so the rest of the codebase keeps its
uniform failure channel
### `extendEnv: true` HACK
Effect's `ChildProcess.make` defaults `extendEnv` to undefined,
which means the spawned process inherits NO environment variables
(empty PATH → `spawn git ENOENT` even when git is on the user's
PATH). `spawnSync` had the opposite default. Setting
`extendEnv: true` restores the historical behavior; commented in
the source as a HACK so future readers don't burn an hour on it.
### Async cascade
Every Git service method already returned `Effect<X, ReactDoctorError>`,
but the production implementation was sync (`Effect.try(spawnSync(...))`).
The new implementation is truly async via `ChildProcess`, so the
legacy sync façades — `getDiffInfo`, `getStagedSourceFiles`,
`materializeStagedFiles`, `neutralizeDisableDirectives` — flip from
`Effect.runSync` to `Effect.runPromise` and become `Promise`-returning.
Production callers (`cli/commands/inspect.ts`, `run-oxlint.ts`)
were already inside async functions, so the only changes are added
`await`s. Tests (`diff-detached-head.test.ts`, `scan-resilience.test.ts`)
get the same `await` treatment and switch their `.toBeNull()` /
`.toEqual(...)` / `.toThrow(...)` assertions to the Promise-aware
counterparts (`resolves.toBeNull`, `resolves.toEqual`, `rejects.toThrow`).
### Dependency
Adds `@effect/platform-node@4.0.0-beta.70` to `@react-doctor/core`
(matches the existing `effect` version pin). The Node platform
services pull in `FileSystem` + `Path` (transitively required by
`NodeChildProcessSpawner`); the bundle stays small because we
import it as `import * as NodeServices from "@effect/platform-node/NodeServices"`
and let the dependency stay external (`neverBundle`).
Co-authored-by: Cursor <cursoragent@cursor.com>
Aiden Bai