Skip to content

fix(effect-needs-cleanup): prove effect-owned cleanup - #1559

Open
skoshx wants to merge 11 commits into
mainfrom
cursor/triage-1558-e58a
Open

fix(effect-needs-cleanup): prove effect-owned cleanup#1559
skoshx wants to merge 11 commits into
mainfrom
cursor/triage-1558-e58a

Conversation

@skoshx

@skoshx skoshx commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Why

effect-needs-cleanup still reports safe effects and accepts unsafe effects when cleanup ownership is expressed through returned listener disposers, mapped subscription collections, or guarded effect-local timer helpers. This builds on #1564 with the adversarial ownership cases that were not covered there.

The need remains reproducible on current main: its copied adversarial suite fails 70 of 160 cases.

What changed

  • Accept callable disposers returned by non-DOM addEventListener and addListener APIs while preserving DOM, legacy media-query, and React Native subscription-object diagnostics.
  • Prove mapped disposer/subscription collections are cleaned exhaustively through forEach or for...of, rejecting conditional cleanup, the wrong collection, escaped collections, and dropped or overwritten entries.
  • Follow directly invoked effect-local helpers and synchronous iterator callbacks when checking collection mutations; ignore uninvoked helpers.
  • Accept effect-owned nested timers only when handle storage, callback ownership, invocation paths, assignments, overwrite protection, and cleanup coverage are all proven.
  • Reject async/deferred owners, sibling-effect timers, imported DOM wrappers, cleanup-after-reschedule patterns, and incomplete ownership.
  • Expand the fuzz generator, verdict-preserving mutations, regression corpus, and false-positive hunter for these patterns.

Validation

Check Result
Current-main necessity reproduction 70 failed / 90 passed in the 160-case adversarial suite
Focused issue #1558 adversarial suite 196/196 passed
Strict targeted fuzz 25,000 seeded generated/mutated cases with invariants and strict mode; passed
Exact-base local RDE 100 project roots across 8 repositories; 51 vs. 51 target-rule diagnostics; identical canonical digest
Full local gates test, lint, typecheck, format check, and JSON smoke passed
GitHub CI Running on 14136e860
Daytona parity Not available in this environment; the exact same-corpus local RDE comparison above completed

Test plan

  • nr test
  • nr lint
  • nr typecheck
  • nr format:check
  • nr smoke:json-report
  • nr -C packages/oxlint-plugin-react-doctor test effect-needs-cleanup.issue-1558.test.ts
  • FUZZ_RULE=effect-needs-cleanup FUZZ_ITERATIONS=25000 FUZZ_SEED=8675309 FUZZ_INVARIANTS=1 FUZZ_STRICT=1 nr fuzz

cursoragent and others added 2 commits August 2, 2026 18:17
…stener/addListener

Fixes #1558 - Part 1

- Allow returning identifiers bound to addEventListener/addListener results as valid cleanup
- Recognize calling such identifiers (e.g., ) as cleanup
- This fixes false positives for React Native's NetInfo.addEventListener and similar APIs

Remaining work: helper function cleanup and array collection cleanup

Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
@pkg-pr-new

pkg-pr-new Bot commented Aug 2, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 14136e8

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Interactive terminal E2E

React Doctor interactive terminal recording

Recorded from the built CLI at 6beed44 in a real terminal. The fixture holds Git busy for three seconds, so Scanning... must appear immediately after project selection, then exercises the compact interactive report.

Download the GIF and MP4 artifact

@aidenybai
aidenybai marked this pull request as ready for review August 3, 2026 03:01
@aidenybai aidenybai changed the title fix(effect-needs-cleanup): recognize returned identifiers for addEventListener/addListener fix(effect-needs-cleanup): prove effect-owned cleanup Aug 3, 2026
@aidenybai

Copy link
Copy Markdown
Member

Superseded by #1564, which carries the current implementation on top of main, includes the cleanup path-coverage review fix, and has the focused/fuzz validation. Closing this older failing-CI branch to avoid merging duplicate detector changes.

@aidenybai aidenybai closed this Aug 3, 2026
@aidenybai aidenybai reopened this Aug 3, 2026
@aidenybai

Copy link
Copy Markdown
Member

Superseded by #1564, which landed the cleanup-ownership fix with focused regression coverage and the FPFNFIX82 ship checks.

@aidenybai aidenybai closed this Aug 3, 2026
@aidenybai aidenybai reopened this Aug 3, 2026
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.

3 participants