Skip to content

fix(effect-needs-cleanup): recognize owned replacement timers - #1826

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1789866878-replacement-timer-cleanup
Open

devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1789866878-replacement-timer-cleanup

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix the confirmed effect-needs-cleanup false positive where an effect-owned event callback replaces one mutable timer from multiple branches:

clearTimeout(timer);
timer = setTimeout(update, delay);

hasOnlySafeHandleStorageAssignments accepts these replacements when the same callback contains an immediately preceding matching global clear in the same block. Existing ownership checks still require release of the final handle and teardown of the callback's registrations on every cleanup path. Callback references must belong to an actual registration or matching removal; passing the callback to an unknown scheduler remains unsafe.

Adds 20 focused cases, minimized passing/failing fuzz fixtures, and a patch changeset. Controls retain warnings for missing releases, overwrite-before-clear, one clear followed by two allocations, incomplete cleanup, deferred/uncalled nested cleanup, shadowed clear functions, escaped callbacks, and async callbacks. Fixture line endings are normalized before constructing the adversarial cases so their multiline replacements also apply on Windows.

Pinned evidence and reproduction

  • Consolidated confirmed_fp candidate: 4b937bcd39e3b0e16086247a80946e7af504bf3b875c9b2742341d42842fef2f.
  • [Pinned overlay at line 203](https://github.com/[REDACTED SECRET]co/expect/blob/39e97500725783490136a8fc7040e6e4dbaafa44/packages/browser/src/runtime/overlay/index.tsx#L203): reposition is registered for scroll/resize, clears the previous timer before either allocation, and returned cleanup removes both listeners and clears the final timer.
  • Exact pinned source SHA-256: 1b4ed40c8fdae899aa509ef92e17a9c048aecd749720def4b3cb3f9f99aa9713.
  • Direct runRule(effectNeedsCleanup, pinnedSource) replay: 1 diagnostic at line 203 before → 0 after, with zero parse errors in both runs.
  • Base: 260b448cdd742e9e62d7029a8d69b0410732ee52; detector package is identical to audited revision 7bfc51485d03ba329fc9033c080e580abe0e3534.

Validation

  • Detector package nr test effect-needs-cleanup: 1,042 passed.
  • Full detector package nr test: 27,153 passed, 187 skipped across 1,085 files.
  • Fuzz package nr test: 217 passed, 907 skipped (the randomized suite is opt-in).
  • FUZZ_RULE=effect-needs-cleanup FUZZ_STRICT=1 FUZZ_ITERATIONS=500 FUZZ_REQUIRE_FIRE=1 FUZZ_PRINT_STATS=1 nr fuzz: passed.
  • Root nr typecheck, nr build, nr lint, nr format:check, nr smoke:json-report: passed; lint emits warnings.
  • Windows CI initially exposed five no-op multiline fixture mutations under CRLF. Reproduced those five failures locally with a CRLF fixture; after normalization all 20 cases pass with CRLF, and all 1,042 rule tests pass with LF. Package typecheck, root lint and format checks passed again after this test-only correction.
  • Built CLI changed-scope scan against the pinned base: zero diagnostics.
  • Local RDE run path:<checkout> --runner local --repositories <pinned-descriptor> --take 1: 1 repository / 1 project root, no errors, zero target-rule diagnostics. The six remaining diagnostics concern other rules; no repositories skipped.
  • Root nr test is not fully green: install-agent-hooks.test.ts / “exits quietly when no react-doctor runner is available” fails because the hook obtains a runner and emits output. Reproduced the identical failure in a detached worktree at the pinned base using nr test tests/install-agent-hooks.test.ts -t 'exits quietly when no react-doctor runner is available'. Core tests passed (2,534); the detector package was separately rerun to completion after Turbo stopped it on the CLI failure. No existing tests were weakened.

Limits

The new proof is deliberately limited to adjacent direct clear/assignment statements in one callback block. It does not infer arbitrary helper-based clears or interprocedural replacement ownership.

Daytona parity pending coordinated validation. The workflow's parity agent owns shared corpus validation; no Daytona run was launched here. No browser/UI testing was requested.

Link to Devin session: https://app.devin.ai/sessions/67080d5c0fa74f1f864f05fa068456b3
Open in Devin Desktop: https://app.devin.ai/desktop/session/67080d5c0fa74f1f864f05fa068456b3?variant=devin

Final coordinated validation

Exact head 6f3c9318: raw +0/-1. The sole removed occurrence is the assigned replacement-timer FP; no other diagnostics changed.

Every detector comparison used the same exact 100 repositories / 100 project roots, complete reports, zero skipped projects, immutable producer/corpus checks, and verified Daytona resource cleanup. Report · Machine-readable rollup · Raw artifacts. This section supersedes earlier pending-Daytona statements. No PR was merged.

Link to Devin session: https://app.devin.ai/sessions/490c477d82c54b9786c13f1d5b79ab35
Open in Devin Desktop: https://app.devin.ai/desktop/session/490c477d82c54b9786c13f1d5b79ab35?variant=devin

Co-Authored-By: aiden@million.dev <aiden.bai05@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

Original prompt from Devin Bot

The user explicitly requested: "go spoin up PRs to fix these iussuse",
referring to our completed FP miner audit. Implement and open a focused PR in github.com/millionco/react-doctor;
do not merely propose fixes. You run on your own VM. Use a new devin/$(date +%s)-<slug>
branch. Never merge, amend commits, skip hooks, change security policy, lower classification
thresholds, or special-case corpus repositories/names. No browser/UI tests are requested.

ATTACHMENT:"https://app.devin.ai/attachments/bc4098da-9223-49de-a3b2-d345607ea2a5/react-doctor-fpfn-review.tar.gz"
ATTACHMENT:"https://app.devin.ai/attachments/81438747-2cfd-49da-8f39-9fa09a5e53ef/react-doctor-fpfn-run.tar.gz"
The review archive has consolidated adjudications and original reviewer reports. Only
consolidated confirmed_fp records are semantic FP evidence; original reviewer labels also
included scan-policy artifacts. The run archive has exact source code, candidate IDs,
detector provenance, original probabilities and all 100 pinned scan records. Extract outside
the repo and inspect relevant parts. Reproduce before fixing, use pinned source, and retain
positive/adversarial controls. Never treat Jev's label as ground truth. User authorized tests
and regression additions required to fix these bugs, not deletion/weakening of old tests.

Read AGENTS.md and applicable repository skills. For detector work invoke rule-writing,
rule-validate, fuzz, and rde-eval, using compact existing contracts. Use ni/nr, truffler before
and after helpers, focused tests plus required lint/typecheck/format, meaningful fuzz
regressions and a patch changeset via nr changeset for published detector changes.
Keep edits minimal and only in your assigned domain. New regression/fuzz files and changeset
filenames must have your unique unit slug so independent PRs do not collide. For registration
files use generators, not manual edits. Inspect existing open fixes before duplicating work.

Push and use builtin fetch_pr_template + git_create... (2501 chars truncated...)

@pkg-pr-new

pkg-pr-new Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 6f3c931

@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Interactive terminal E2E

Terminal Control verified the built CLI at 6f3c931 in a real PTY:

  • selected a project interactively and observed Scanning... before the three-second Git delay completed
  • waited for the clean result and exercised the compact report
  • opened copy context and the GitHub Actions confirmation, then cancelled safely

Download the edited MP4 and PNG evidence

Co-Authored-By: aiden@million.dev <aiden.bai05@gmail.com>

This branch has not been deployed

No deployments
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.

0 participants