Skip to content

fix: pre-commit hook now blocks commits when diagnostics are found#970

Open
skoshx wants to merge 2 commits into
mainfrom
cursor/triage-969-6a85
Open

fix: pre-commit hook now blocks commits when diagnostics are found#970
skoshx wants to merge 2 commits into
mainfrom
cursor/triage-969-6a85

Conversation

@skoshx

@skoshx skoshx commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Root Cause

The generated pre-commit hook captured react-doctor output to a temp file but never exited with non-zero status when diagnostics were found. This allowed commits to proceed even when blocking diagnostics were present, completely defeating the purpose of the hook.

The bug existed in two places:

  1. buildReactDoctorHookBlock() in install-git-hook-file.ts
  2. NON_BLOCKING_REACT_DOCTOR_COMMAND in git-hook-shared.ts

Both implementations had the same flaw:

  • Captured output to a temp file
  • Deleted the file in both success and failure branches
  • Never called exit 1 in the failure branch
  • Printed a generic message but allowed the commit to proceed

Fix

The hook now:

  1. Displays the full react-doctor output to stderr (so users see which rules failed)
  2. Exits with status 1 when diagnostics are found (blocking the commit)
  3. Cleans up the temp file after displaying output

Testing

  • Updated existing integration test that was expecting non-blocking behavior
  • Test now verifies the commit is blocked (exit code 1) when react-doctor fails
  • Test verifies react-doctor output is displayed to the user
  • All existing tests pass

Closes #969

Open in Web Open in Cursor 

cursoragent and others added 2 commits June 25, 2026 17:45
The hook previously captured react-doctor output to a temp file, deleted it,
and never exited with non-zero status. This allowed commits to proceed even
when blocking diagnostics were found, defeating the entire purpose of the hook.

Changes:
- Hook now displays react-doctor output before exiting
- Hook exits with status 1 when diagnostics are found
- Updated tests to expect blocking behavior
- Fixed test names to remove "non-blocking" references

Fixes #969

Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
@cursor cursor Bot marked this pull request as ready for review June 25, 2026 17:47
@pkg-pr-new

pkg-pr-new Bot commented Jun 25, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 302cb0b

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.

pre-commit hook swallows diagnostics and does not block commits

2 participants