Add React Doctor to GitHub Actions - #20
Conversation
…tionErrorBanner, merge file icon utilities
…tationErrorBanner, and useCrudDialogs hook
…ationErrorBanner, standard UI fields, and useCrudDialogs hook
…lide-up-fade in settings.tsx
…e-pass loops instead of map().filter() chains
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 21 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds React Doctor CI workflow and config; introduces shared UI primitives ( ChangesMonolithic PR: tooling, shared UI, charts, routes, DB
Sequence Diagram(s)(since the PR adds a CI workflow that invokes checkout then runs react-doctor, a small sequence diagram is included above in the hidden artifact) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
React Doctor found no issues. 🎉 Reviewed by React Doctor for commit |
Greptile SummaryThis PR integrates React Doctor as a CI check on every PR and push to
Confidence Score: 5/5Safe to merge; changes are additive CI infrastructure plus formatting auto-fixes with no functional regressions The workflow and config are new additive files, the dependency removals are confirmed safe (web-vitals is not imported anywhere; @tanstack/router-plugin remains as a transitive dep and is not referenced in vite.config.ts), and all source-file changes are React Doctor's own reformatting output with no altered runtime logic doctor.config.json is worth a second look to confirm the 12 disabled rules represent intentional suppression and not accidental over-silencing of the scan Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR opened or push to main] --> B{Event type}
B -- pull_request --> C[Cancel in-flight scan for same PR]
B -- push to main --> D[Cancel in-flight scan for same ref]
C --> E[checkout]
D --> E
E --> F[millionco/react-doctor v2]
F --> G[Read doctor.config.json]
G --> H{Scan scope}
H -- PR --> I[Changed files vs merge base]
H -- push --> J[Full project health scan]
I --> K[Sticky PR comment plus inline comments plus commit status]
J --> L[Health score trend plus commit status]
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
package.json:16
The `doctor` script fetches and executes `react-doctor@latest` on every invocation via `npx`, so developers on the same codebase can get different scan results depending on when they run it. Pinning to the same major version used by the CI workflow (`@2`) keeps local runs consistent with CI.
```suggestion
"doctor": "npx react-doctor@2"
```
Reviews (2): Last reviewed commit: "Merge branch 'main' into react-doctor/ad..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| steps: | ||
| - uses: actions/checkout@v5 | ||
|
|
||
| - uses: millionco/react-doctor@v2 |
There was a problem hiding this comment.
millionco/react-doctor@v2 is a mutable tag — if the publisher force-pushes to that tag (intentionally or after a supply-chain compromise), the updated code will run automatically on every future PR with the pull-requests: write, issues: write, and statuses: write permissions granted here. GitHub's hardened-runner guidance and OSSF Scorecard both require pinning third-party actions to an immutable full-length commit SHA (e.g. millionco/react-doctor@<40-char-sha>). The commented-out version: input pins the react-doctor binary version but not the action's own JavaScript runner, so it does not close this gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/react-doctor.yml
Line: 34
Comment:
**Third-party action not pinned to a commit SHA**
`millionco/react-doctor@v2` is a mutable tag — if the publisher force-pushes to that tag (intentionally or after a supply-chain compromise), the updated code will run automatically on every future PR with the `pull-requests: write`, `issues: write`, and `statuses: write` permissions granted here. GitHub's hardened-runner guidance and OSSF Scorecard both require pinning third-party actions to an immutable full-length commit SHA (e.g. `millionco/react-doctor@<40-char-sha>`). The commented-out `version:` input pins the *react-doctor binary* version but not the action's own JavaScript runner, so it does not close this gap.
How can I resolve this? If you propose a fix, please make it concise.| react-doctor: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 |
There was a problem hiding this comment.
actions/checkout also uses a mutable tag
actions/checkout@v5 points to a mutable version tag. While GitHub's own actions are lower risk than third-party ones, pinning to the full SHA is still the recommended practice for supply-chain hardness. Resolve the SHA with gh api /repos/actions/checkout/git/ref/tags/v5 (or check the release page) and pin it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/react-doctor.yml
Line: 32
Comment:
**`actions/checkout` also uses a mutable tag**
`actions/checkout@v5` points to a mutable version tag. While GitHub's own actions are lower risk than third-party ones, pinning to the full SHA is still the recommended practice for supply-chain hardness. Resolve the SHA with `gh api /repos/actions/checkout/git/ref/tags/v5` (or check the release page) and pin it.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/react-doctor.yml:
- Around line 32-34: The workflow uses mutable tags actions/checkout@v5 and
millionco/react-doctor@v2 and leaves checkout credentials persisted; update the
checkout step to pin actions/checkout to its full commit SHA and replace
millionco/react-doctor@v2 with its corresponding full commit SHA, and add
persist-credentials: false to the checkout step to avoid persisting GITHUB_TOKEN
in git config; locate these tokens by searching for "actions/checkout@v5",
"millionco/react-doctor@v2", and the checkout step block (persist-credentials)
in the react-doctor.yml and replace accordingly.
- Line 32: The checkout step using "uses: actions/checkout@v5" currently
persists credentials; update that GitHub Actions step to include a with block
setting persist-credentials: false so the checkout token/SSH key is not written
to the runner git config (i.e., modify the step that references
actions/checkout@v5 to add "with: persist-credentials: false").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdb5ddd9-49d4-4911-b2d1-0503fca36c8f
📒 Files selected for processing (1)
.github/workflows/react-doctor.yml
…arnings, hoist default arrays, and improve accessibility
…olation and fix LoadingSankey definition
…ngs in routes and forms
…ctor script version to v2
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/settings.tsx (1)
166-184: 💤 Low value
isDefaultSettingsused inside effect but not listed in dependencies.The effect uses
isDefaultSettingswhich is derived outside the effect. While this works because the underlying values (workspaceNameVal,supportEmailVal) are in the dependency array, this pattern may triggerexhaustive-depslint warnings and creates an implicit dependency that could confuse maintainers.Consider adding to dependency array for explicitness
}, [ isDefaultSettings, + isDefaultSettings, hasSettings, hasUser, currentUserName, currentUserEmail, workspaceNameVal, supportEmailVal, ]);Or compute
isDefaultSettingsinside the effect body for self-contained logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/settings.tsx` around lines 166 - 184, The effect in the Settings component references the external variable isDefaultSettings but doesn't include it in the dependency array, causing an implicit dependency and potential lint warnings; either add isDefaultSettings to the dependency array for the useEffect that sets workspaceName/supportEmail, or move the isDefaultSettings computation into the effect body so the effect is self-contained (update the useEffect surrounding setWorkspaceName/setSupportEmail and the dependency list accordingly).src/db/records.ts (1)
645-655: ⚡ Quick winConsider splitting the validation into separate, explicit checks.
The combined condition
if (!(project && isValidTransition))conflates two distinct error cases: a missing project and a no-op status change. Separating these would improve clarity and allow more specific error messages.♻️ Suggested refactor for clarity
- const isValidTransition = - project && project.status !== updated.requestedStatus; - - if (!(project && isValidTransition)) { + if (!project) { + console.error( + `Status change approval failed: Project ${updated.projectId} not found.` + ); + throw new Error(`Project ${updated.projectId} not found`); + } + + if (project.status === updated.requestedStatus) { console.error( - `Status change approval failed: Project ${updated.projectId} missing or invalid transition to ${updated.requestedStatus}.` + `Status change approval failed: Project ${updated.projectId} is already in ${updated.requestedStatus} status.` ); - throw new Error( - `Invalid status transition to ${updated.requestedStatus}` - ); + throw new Error(`Project is already in ${updated.requestedStatus} status`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/db/records.ts` around lines 645 - 655, Split the combined conditional that currently reads if (!(project && isValidTransition)) into two explicit checks: first verify the existence of project and throw a clear "Project not found" (or similar) error if missing, then separately check isValidTransition and throw a distinct "No-op status change" or "Invalid status transition" error if the transition is not allowed; update the function that performs the status update (the block referencing project and isValidTransition) so it uses these two checks and returns/throws the more specific error messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/common/state-panel.tsx`:
- Around line 58-69: The MutationErrorBanner currently renders errors in a plain
<p> so screen readers may not announce new messages; update the
MutationErrorBanner function to add live-region semantics to the banner element
by setting role="alert" and aria-live="assertive" (and optionally
aria-atomic="true") on the rendered paragraph element so assistive tech will
announce new mutation errors immediately while keeping the existing styling and
structure.
---
Nitpick comments:
In `@src/db/records.ts`:
- Around line 645-655: Split the combined conditional that currently reads if
(!(project && isValidTransition)) into two explicit checks: first verify the
existence of project and throw a clear "Project not found" (or similar) error if
missing, then separately check isValidTransition and throw a distinct "No-op
status change" or "Invalid status transition" error if the transition is not
allowed; update the function that performs the status update (the block
referencing project and isValidTransition) so it uses these two checks and
returns/throws the more specific error messages.
In `@src/routes/settings.tsx`:
- Around line 166-184: The effect in the Settings component references the
external variable isDefaultSettings but doesn't include it in the dependency
array, causing an implicit dependency and potential lint warnings; either add
isDefaultSettings to the dependency array for the useEffect that sets
workspaceName/supportEmail, or move the isDefaultSettings computation into the
effect body so the effect is self-contained (update the useEffect surrounding
setWorkspaceName/setSupportEmail and the dependency list accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d50e8dc2-d5d4-490d-b175-3e97cc5b0613
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.github/workflows/react-doctor.ymldoctor.config.jsonpackage.jsonscripts/migrate-portal.tssrc/__tests__/api-invite-management.test.tssrc/components/auth/auth-shell.tsxsrc/components/auth/worker-invite-form.tsxsrc/components/common/panel-section.tsxsrc/components/common/product-charts.tsxsrc/components/common/route-pending.tsxsrc/components/common/state-panel.tsxsrc/components/evilcharts/charts/bar-chart.tsxsrc/components/evilcharts/charts/line-chart.tsxsrc/components/evilcharts/charts/radial-chart.tsxsrc/components/evilcharts/charts/sankey-chart.tsxsrc/components/evilcharts/ui/chart.tsxsrc/components/evilcharts/ui/evil-brush.tsxsrc/components/evilcharts/ui/legend.tsxsrc/components/evilcharts/ui/tooltip.tsxsrc/components/layout/portal-shell.tsxsrc/components/projects/project-collaboration-panel.tsxsrc/components/projects/project-files-panel.tsxsrc/components/projects/project-milestones-panel.tsxsrc/components/projects/project-updates-panel.tsxsrc/db/records.tssrc/db/schema.tssrc/lib/api.tssrc/lib/insights.tssrc/lib/utils.tssrc/routes/api/admin/status-change-requests.tssrc/routes/api/admin/status-change-requests/$id.tssrc/routes/api/clients/$id/invites.tssrc/routes/api/invites/$id/approve.tssrc/routes/api/portal/activity.tssrc/routes/api/portal/files.tssrc/routes/api/portal/status-change-requests.tssrc/routes/api/portal/team.tssrc/routes/clients/$id.tsxsrc/routes/dashboard.tsxsrc/routes/portal/activity.tsxsrc/routes/portal/files.tsxsrc/routes/portal/index.tsxsrc/routes/portal/projects/$id.tsxsrc/routes/portal/team.tsxsrc/routes/projects/$id.tsxsrc/routes/settings.tsxsrc/styles.css
✅ Files skipped from review due to trivial changes (14)
- src/routes/api/admin/status-change-requests.ts
- src/routes/api/clients/$id/invites.ts
- doctor.config.json
- src/routes/api/portal/activity.ts
- scripts/migrate-portal.ts
- src/components/evilcharts/charts/line-chart.tsx
- src/styles.css
- src/tests/api-invite-management.test.ts
- src/routes/api/invites/$id/approve.ts
- src/db/schema.ts
- src/routes/clients/$id.tsx
- src/routes/portal/activity.tsx
- src/routes/portal/team.tsx
- src/components/evilcharts/charts/bar-chart.tsx
… to formatter in tooltip, and exclude react-doctor-report.json in gitignore
1 participant
Adds a React Doctor scan to every pull request and every push to the default branch. The workflow file is documented inline.
Docs: https://www.react.doctor/ci
Summary by CodeRabbit
Chores
New Features
UI Updates
Accessibility