fix(effect-needs-cleanup): recognize owned replacement timers - #1826
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Co-Authored-By: aiden@million.dev <aiden.bai05@gmail.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
Original prompt from Devin Bot
|
commit: |
Interactive terminal E2ETerminal Control verified the built CLI at
|
Co-Authored-By: aiden@million.dev <aiden.bai05@gmail.com>
Summary
Fix the confirmed
effect-needs-cleanupfalse positive where an effect-owned event callback replaces one mutable timer from multiple branches:hasOnlySafeHandleStorageAssignmentsaccepts 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
confirmed_fpcandidate:4b937bcd39e3b0e16086247a80946e7af504bf3b875c9b2742341d42842fef2f.repositionis registered for scroll/resize, clears the previous timer before either allocation, and returned cleanup removes both listeners and clears the final timer.1b4ed40c8fdae899aa509ef92e17a9c048aecd749720def4b3cb3f9f99aa9713.runRule(effectNeedsCleanup, pinnedSource)replay: 1 diagnostic at line 203 before → 0 after, with zero parse errors in both runs.260b448cdd742e9e62d7029a8d69b0410732ee52; detector package is identical to audited revision7bfc51485d03ba329fc9033c080e580abe0e3534.Validation
nr test effect-needs-cleanup: 1,042 passed.nr test: 27,153 passed, 187 skipped across 1,085 files.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.nr typecheck,nr build,nr lint,nr format:check,nr smoke:json-report: passed; lint emits warnings.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.nr testis 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 usingnr 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