Skip to content

feat(action): React Doctor Review composite action#307

Merged
aidenybai merged 20 commits into
mainfrom
cursor/pr-review-action-b4f1
May 23, 2026
Merged

feat(action): React Doctor Review composite action#307
aidenybai merged 20 commits into
mainfrom
cursor/pr-review-action-b4f1

Conversation

@aidenybai
Copy link
Copy Markdown
Member

Mirrors the hosted react.review bot's doctor-only review pipeline as a self-hosted GitHub Action that lives in this repo. Consumers add it to their workflow and get inline doctor diagnostics, a sticky summary comment, thread reconciliation, and a check run — driven by their own CI, no hosted service required.

What ships

  • action.yml at the repo root — new composite action: setup-node → pnpm install (filtered to @react-doctor/review-action... + react-doctor... to keep cold-start install minimal) → pnpm --filter react-doctor buildtsx actions/review/src/index.ts. Inputs: token (defaults to ${{ github.token }}), directory, node-version.
  • actions/review/ — new workspace package @react-doctor/review-action with three TypeScript modules co-located per the AGENTS.md "resist over-splitting" guideline:
    • src/pipeline.ts — pure helpers: unified-diff parser, runDiagnoseAcrossWorkspace (monorepo fan-out + weighted score + path remap), computeDiagnosticsDelta (count-delta keyed by (relativePath, rule, message)), inline-candidate builder, comment formatters (formatPendingReviewComment / formatNoIssuesComment / formatRegressionComment / formatAnalysisFailureComment), and getReviewCheckAssessment for the check-run body.
    • src/github.ts — Octokit IO: compareCommitsWithBasehead (with ${headOwner}:${headRef} for fork PRs), paginated pulls.listFiles, sticky-comment upsert/delete, pulls.createReview for inline comments, GraphQL reviewThreads pagination + resolveReviewThread mutation, check-run create/complete.
    • src/index.ts — entrypoint that wires everything together.
    • src/constants.ts — magic numbers (MAX_INLINE_COMMENTS_COUNT = 25, sticky/inline markers, check-run name, GraphQL page size) per the AGENTS.md SCREAMING_SNAKE_CASE rule.
  • actions/inspect/action.yml — the previous "scan + sticky comment" composite action moved here so existing consumers can pin to millionco/react-doctor/actions/inspect@main without breaking.
  • .github/workflows/doctor-action.yml — dogfood workflow that runs the action against this repo's own PRs via uses: ./.
  • README updates — new "PR Review (recommended)" section plus a "Scan + sticky comment (legacy)" subsection. All previous millionco/react-doctor@main references for the legacy inputs are repointed to millionco/react-doctor/actions/inspect@main.

What the action does on every PR

On pull_request (opened / synchronize / reopened):

  1. Reads the event payload from GITHUB_EVENT_PATH, extracts owner/repo/PR number/head SHA/base ref, and detects fork PRs via head.repo.full_name vs base.repo.full_name.
  2. Resolves the merge base via octokit.repos.compareCommitsWithBasehead.
  3. Materializes the base commit alongside the runner's head checkout 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.
  4. Lists changed files via octokit.pulls.listFiles (paginated). Each file's addedLineContents: Map<lineNumber, content> is derived from the unified-diff patch — required for resolving which diagnostic lines landed on + lines.
  5. Runs react-doctor/api's diagnose() in-process on both the head checkout and the base worktree. Pre-discovers React subprojects via discoverReactSubprojects (workspaces in package.json + pnpm-workspace.yaml), fans out per project, combines diagnostics with paths remapped to be relative to the repo root, and weight-averages scores by sourceFileCount.
  6. Diffs head vs base diagnostics using a (relativePath, rule, message) key with count deltas (not boolean presence) so duplicate diagnostics are counted correctly.
  7. Inline candidates are net-new diagnostics with severity === "error" whose line lives in the file's addedLineContents map.
  8. Reconciles existing inline threads via the GraphQL repository.pullRequest.reviewThreads query:
    • Threads matching a current candidate (by parsed threadKey in the HTML marker) are marked active.
    • Threads no longer in the candidate set get ✅ Addressed in <headSha> appended via pulls.updateReviewComment and resolved via the resolveReviewThread mutation.
  9. Posts remaining (non-active) candidates via octokit.pulls.createReview({ event: "COMMENT" }), capped at MAX_INLINE_COMMENTS_COUNT (25) per run.
  10. Upserts the sticky summary comment by <!-- react-doctor-review --> marker. A pending-review comment is posted before analysis starts; it's swapped for either formatRegressionComment (regressions found) or formatNoIssuesComment (clean PR) once diagnostics are computed, with a doctor metrics table in a <details> block. On catch, falls back to formatAnalysisFailureComment.
  11. Creates a check run on the head SHA before analysis, then completes it with the assessment body — score table, new-diagnostics list, project metadata.

Edge cases handled

  • Fork PRshead.repo.full_name !== base.repo.full_name triggers a warning, and write failures on inline / sticky posts are caught (the workflow GITHUB_TOKEN is read-only on fork PRs). Consumers can pass a PAT or App installation token via the token input to unlock fork-PR review.
  • Diagnostic path resolution — tries absolute under the repo root, absolute under the project root, relative + project-prefixed, and "already prefixed" in that order so monorepo packages report under the correct workspace.
  • NoReactDependencyError / PackageJsonNotFoundError / ProjectNotFoundError — the check run concludes as skipped with a "not a React project" summary and the pending sticky comment is deleted. Doesn't loud-fail.
  • Same diagnostic occurring multiple times — the diff uses count deltas keyed by (relativePath, rule, message), picking head occurrences in order.
  • Inline comment markers — every inline body is prefixed with <!-- react-doctor-review-inline:<threadKey> --> so the next run can parse the marker out of the GraphQL response and decide whether to skip, resolve, or post.
  • Base worktree cleanupgit worktree remove --force runs in a finally block.

Token strategy (dual-mode)

  • Default — workflow GITHUB_TOKEN (action.yml default). Comments author as github-actions[bot]. Works for same-repo PRs; read-only on fork PRs (warning logged, posts attempted).
  • token input override — consumer passes a PAT or GitHub App installation token via secrets for fork-PR support or branded identity.

OIDC token-broker mode is intentionally not in scope for v1 — that flow lives in the hosted react.review service and shouldn't couple this standalone action to it.

Out of scope (intentionally)

  • No LLM / agent layer (the hosted bot's agent-review.ts is not ported — this is doctor-only review).
  • No Vercel Sandbox dependency (everything runs in-process on the runner).
  • No Upstash rate limit / no-react cache (multi-tenant guardrails the hosted service needs, irrelevant for single-repo CI).
  • No proprietary react.review URLs in the comment body — branding is react-doctor-flavored (_Reviewed by [react-doctor](https://github.com/millionco/react-doctor) — local CI, no hosted service._).

Definition of done

A consumer adds:

permissions:
  contents: read
  pull-requests: write
  checks: write
  issues: write
steps:
  - uses: actions/checkout@v5
    with: { fetch-depth: 0 }
  - uses: millionco/react-doctor@main

…and on their next PR sees inline comments on net-new violations, a sticky summary with the doctor metrics table, a check run on the head SHA, and resolved threads with ✅ Addressed in <sha> when they push fixes.

Verification

  • pnpm typecheck
  • pnpm lint
  • pnpm format:check
  • pnpm test ✓ (existing github-action.test.ts contract test repointed at actions/inspect/action.yml)
  • Smoke-tested the entrypoint locally to confirm imports resolve and the missing-event error path is handled gracefully.
Open in Web Open in Cursor 

@reactreview
Copy link
Copy Markdown

reactreview Bot commented May 21, 2026

0 score

Copy as prompt
Check if these React Review issues are valid. If so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.

Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff

Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issues instead of changing or suppressing the rules.

React Review found 0 errors and 5 warnings. This PR leaves the React health score unchanged.

<file name="actions/review/src/pipeline.ts">

<violation number="1" location="actions/review/src/pipeline.ts:207">
Severity: Warning

await inside a for…of loop runs the calls sequentially — for independent operations, collect them and use `await Promise.all(items.map(...))` to run them concurrently

Collect the items and use `await Promise.all(items.map(...))` to run independent operations concurrently

Rule: `async-await-in-loop`
</violation>

<violation number="2" location="actions/review/src/pipeline.ts:225">
Severity: Warning

result.project.sourceFileCount is read 3 times inside this loop — hoist into a const at the top of the loop body

Hoist the deep member access into a const at the top of the loop body: `const { x, y } = obj.deeply.nested`

Rule: `js-cache-property-access`
</violation>

</file>

<file name="actions/review/src/github.ts">

<violation number="1" location="actions/review/src/github.ts:196">
Severity: Warning

await inside a for…of loop runs the calls sequentially — for independent operations, collect them and use `await Promise.all(items.map(...))` to run them concurrently

Collect the items and use `await Promise.all(items.map(...))` to run independent operations concurrently

Rule: `async-await-in-loop`
</violation>

<violation number="2" location="actions/review/src/github.ts:194">
Severity: Warning

array.includes() in a loop is O(n) per call — convert to a Set for O(1) lookups

Use a `Set` or `Map` for repeated membership tests / keyed lookups — `Array.includes`/`find` is O(n) per call

Rule: `js-set-map-lookups`
</violation>

</file>

<file name="actions/review/src/index.ts">

<violation number="1" location="actions/review/src/index.ts:320">
Severity: Warning

.filter().map() iterates the array twice — combine into a single loop with .reduce() or for...of

Combine `.map().filter()` (or similar chains) into a single pass with `.reduce()` or a `for...of` loop to avoid iterating the array twice

Rule: `js-combine-iterations`
</violation>

</file>

Reviewed by reactreview for commit 9c44b26. Configure here.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
react-doctor-website Ready Ready Preview, Comment May 23, 2026 3:29am

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

React Doctor Review

Score: 99/100 - Excellent (unchanged from base)

No new React Doctor regressions in this PR.

Doctor metrics
Project Directory Framework React Files Errors Warnings Score
website packages/website nextjs 19.2.5 17 0 5 99

Reviewed by react-doctor - local CI, no hosted service.

Comment thread actions/review/src/pipeline.ts Outdated
Comment thread actions/review/package.json Outdated
Comment thread actions/review/src/index.ts Outdated
Comment thread actions/review/src/pipeline.ts
Comment thread actions/review/src/constants.ts Outdated
Comment thread actions/review/src/github.ts Outdated
Comment thread actions/review/src/pipeline.ts Outdated
Comment thread actions/review/src/index.ts Outdated
Comment thread actions/review/src/pipeline.ts
Comment thread actions/review/src/index.ts Outdated
Comment thread actions/review/src/pipeline.ts Outdated
Comment thread actions/review/src/index.ts Outdated
Comment thread actions/review/src/index.ts
Comment thread actions/review/src/index.ts
Comment thread actions/review/src/pipeline.ts
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 and found 2 potential issues.

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 8cc2f23. Configure here.

} catch {
// Permission may be insufficient; skip silently.
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Resolves threads when line shifts

Medium Severity

Thread reconciliation treats buildThreadKey as an exact match, including the diagnostic line number. When a push keeps the same rule violation but moves it to another line, the old inline thread’s key no longer appears in the active set, so the action appends “Addressed” and resolves the thread even though the regression still appears in the sticky summary and may get a new inline comment.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8cc2f23. Configure here.

Comment thread actions/review/src/github.ts
cursoragent and others added 19 commits May 22, 2026 20:27
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
…th 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>
… (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>
… 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>
…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>
…s (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>
…e 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>
…ift 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>
@aidenybai aidenybai force-pushed the cursor/pr-review-action-b4f1 branch from 45d727f to 9c44b26 Compare May 23, 2026 03:28
@aidenybai aidenybai merged commit 30f05ef into main May 23, 2026
8 checks passed
@aidenybai aidenybai deleted the cursor/pr-review-action-b4f1 branch May 23, 2026 03:34
aidenybai added a commit that referenced this pull request May 23, 2026
aidenybai added a commit that referenced this pull request May 23, 2026
aidenybai added a commit that referenced this pull request May 23, 2026
README (594 → 394 lines, -34%):
- collapse 3 redundant YAML examples in `PR blocking` to one explanation
- shrink Husky/lint-staged walkthrough to one config block + one warning
- compress `Custom rules` from 60 lines of numbered tutorial to a single example
- merge `Agent and CI integration` bullet list (duplicated CLI ref + PR blocking + Node API)
- collapse 5 prose paragraphs after the config table into a 5-bullet list
- cut implementation-detail bullet list of `rn-no-raw-text` Platform.OS forms
- drop NickvanDyke "we ported these rules" trivia paragraph
- replace duplicated install block in `Resources & Contributing Back` with one line
- inline single-row `Optional companion plugins` table

CONTEXT.md + .specs/inspect-pipeline.md:
- drop `actions/review` / `@react-doctor/review-action` section (reverted in #443)
- replace `@react-doctor/project-info` / `@react-doctor/types` refs with
  `core/src/project-info` / `core/src/types` (folded into core in #440)
- fix "13 leaf reasons today" miscount → 12
- update service count 9 → 11

Deletes:
- TODOS.md (442-line working backlog — belongs in issues, not the repo)
- .agents/skills/thermo-nuclear-code-quality-review/ (200-line skill)
- packages/react-doctor/CHANGELOG.md.bak (68KB tracked backup file)

Validated: `pnpm format`, `pnpm lint`, `pnpm typecheck`, `pnpm test` (1449 pass, 3 skip).

Co-authored-by: Cursor <cursoragent@cursor.com>
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