* fix(ci): harden workflows against cache poisoning and document unsigned tags
- Restrict TURBO_TOKEN/TURBO_TEAM to main-branch pushes only across
ci.yml, code-quality.yml, and react-doctor.yml. First-party PR branches
no longer have cache write access (preventing cache poisoning attacks).
PRs run with a cold cache.
- Add SECURITY comments in action-version-bump.yml explaining that tags
are unsigned and recommending SHA pinning for hardened CI.
- Update install-github-workflow.ts template to include security note
about @v2 floating major tag and SHA pinning recommendation.
Closes#1685
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
* fix(ci): make pull request cache read-only
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Co-authored-by: Aiden Bai <aiden@million.dev>
The 0.7.3 failure was IDENTITY_TOKEN_READ_ERROR mid parallel publish,
not missing registry-url. Adding registry-url writes an empty
_authToken line that blocks Trusted Publishing OIDC
(actions/setup-node#1551). Keep force_publish dispatch to finish
partial releases.
setup-node was missing registry-url on the main publish job, so trusted
publishing failed mid-release (react-doctor@0.7.3 / eslint-plugin). Match
publish-dev, and add workflow_dispatch force_publish to finish partial releases.
* feat(action,core): CI speedups — install cache, persistent scan caches, local diff scope (plans 09–11)
The GitHub Action's dominant cost on a PR run is the uncached install (~15s of
an ~18s step), not the scan. These three plans target the CI experience:
Plan 09 — cache the install (biggest CI win). A resolve-version step pins the
concrete published version (so the cache key is stable even for `latest`;
scripts/resolve-package-spec.mjs), an actions/cache step restores the install
keyed on version+node+os+arch (no fuzzy fallback — native ABI safety), and the
scan installs into the cached `--prefix` only on a miss. A non-cacheable
local-path spec keeps the npx path. ~15s install → ~1-2s restore on a hit.
Plan 11 — derive PR changed files locally + lock the diff fast path. The base
step now runs `git diff --name-only --diff-filter=AMR <base>...HEAD` (faster, no
API rate limit, works on forks), falling back to the GitHub API only when the
base isn't reachable; both share scripts/normalize-changed-files.mjs. A
regression test locks that diff mode skips dead-code + supply-chain (the
fast-path guarantee). A clearer degraded-mode warning points at `fetch-depth: 0`.
Plan 10 — persist scan caches across CI runs. `REACT_DOCTOR_CACHE_DIR` lets the
action point the engine's caches at a stable `${runner.temp}` path an
actions/cache step persists, so the per-file content-addressed lint cache (#900)
restores across commits — a PR re-lints only its changed files. A new
supply-chain per-PURL on-disk cache (24h TTL, fail-open, NO_CACHE-bypassed) skips
the Socket network for unchanged deps.
Unit/integration tests cover all three (version classification, changed-file
normalization, diff-fast-path skip, cache-dir override, supply-chain cache
hit/bypass); changeset for the npm-facing surface (REACT_DOCTOR_CACHE_DIR +
supply-chain cache). Action releases (tags) + the self-test cacheable-install
job (a workflow-file change needing `workflow` push scope) + R3 sparse-checkout
docs + plan-09's `--print-cache-key` follow-up are noted for a workflow-scoped
push / dogfooding pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(action): self-test the cacheable install path (plan 09)
Adds an `action-cacheable-install` job that runs the action with a published
`version:` (latest) so CI exercises the resolve-version + actions/cache +
prefix-install branch (the local-path job covers the npx branch). Advisory
(`blocking: none`). Split out from the main plan-09 commit because pushing a
`.github/workflows/` change requires `workflow` token scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(cli): migrate mutable @main action refs to @v2 (action change → migration)
Since plans 09-11 change the action, register a once-per-repo project migration
(the framework's "action updates" path) that pins a mutable `@main` / `@master`
React Doctor action reference in `.github/workflows/*.yml` to the recommended
floating major `@v2`.
An unpinned `@main` runs whatever the action's HEAD points to with the
workflow's write permissions — a supply-chain risk (#299) — and the rewrite also
moves the workflow onto the install- and scan-cached release. Only mutable refs
are rewritten; pinned tags / SHAs are deliberate and untouched, a different
action on `@main` is ignored, and only the ref changes (owner, comments, and the
`version:` input are preserved). Runs once per repo like the legacy-config
migration and logs the change for review/commit (or revert if intentionally
tracking main). No-op (stays pending) when there's no mutable ref.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(action): update the action contract test for the plans 09-11 restructure
github-action.test.ts asserts the literal content of action.yml's steps; plans
09/11 moved that content into shared scripts, so two assertions broke:
- the inline `directoryPrefix` prefix-stripping is now in the shared
normalize-changed-files.mjs (used by both the local-diff base step and the API
fallback) — assert the wiring instead, with the behavior locked by
normalize-changed-files.test.ts.
- the inline `PACKAGE_SPEC="react-doctor@$INPUT_VERSION"` derivation moved to the
resolve-version step (resolve-package-spec.mjs) — assert it's read from that
step's output.
Adds a test for the new contract (resolve-version + the toolchain/scan
actions/cache steps + the cached prefix-install). Behavior unchanged; the
contract test now matches the reworked action.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: order the diagnostics dump by rule priority
Users report that `npx react-doctor` prints a large, undifferentiated dump and
they don't know what to tackle first. The score API now returns an intrinsic
priority/tier per rule; surface it so the dump leads with what's most worth
fixing instead of ordering purely by severity.
- core: `ScoreResult` carries the API's `rules: {ruleKey: {priority, tier}}`
payload (new `RulePriority`/`RuleTier` types); `parseScoreResult` captures it.
- CLI: rule-groups and categories sort by `effectivePriority` (API priority
desc). Unranked rules get a severity-based midpoint; with no score at all
(`--no-score` / API unreachable) ordering degrades to the previous
error-before-warning + count order, so there is no regression offline.
No priority table is duplicated here — the engine remains the single source of
truth and the values arrive over the score API.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(cli): improve error handling for command execution failures
* refactor(core): validate the score API response with effect/Schema
Replace the hand-rolled type guards in `parseScoreResult`/`parseRulePriorities`
with `effect/Schema` (the schema lib already used across @react-doctor/core, see
schemas.ts) — `Schema.Struct` + `Schema.Literals` + `Schema.Record` decoded via
`decodeUnknownOption`. A malformed payload decodes to null and falls back to the
severity ordering, same as before. Result types are now `readonly` to match the
decoded shape.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
setup-node's registry-url writes an .npmrc with _authToken placeholder
that short-circuits the OIDC exchange. Also need npm 11.5.1+ for
trusted publishing support.
* 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>
Aiden Bai