Skip to content

FPFNFIXAPEX fix guarded async subscription cleanup - #1959

Open
skoshx wants to merge 3 commits into
mainfrom
cursor/triage-1957-c259
Open

skoshx wants to merge 3 commits into
mainfrom
cursor/triage-1957-c259

Conversation

@skoshx

@skoshx skoshx commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1957.

An effect can wait for availability, stop a late subscription after cancellation, and release its current handle during cleanup. effect-needs-cleanup still reported this pattern because its guarded deferred-cleanup proof covered timers only.

Changes

  • Extend that proof to subscriptions with one directly initiated promise continuation and a cancellation guard invalidated by every cleanup path.
  • Require a proven native promise, a stable async/local producer, an explicit unshadowed Promise return contract, or the documented Expo Accelerometer availability API.
  • Match the current retained handle to its synchronous release. Preserve valid nullable-handle cleanup and reject skipped, throwing, overwritten, wrong-receiver, or reacquiring cleanup.
  • Reject repeated initiation, opaque thenables, mutated promise methods, and named callback recursion or escape.
  • Reconcile the existing PR with current main and retain the original tests and synthetic fixtures.

Validation

Current public head: 41da30539bd91a95087ab0d1cb89f251d21be635.
Tree: 4797f062eadcae328923a55c1651f6cf8bb81e45, identical to signed local merge baad3d839d2f0bc15dc8d72cb1159cd066ebedb0 and prospective merge bca96f52909fb59827387c583b15f1af0d7fdf5a. The GitHub App commit itself is unsigned.

  • 34,984 tests passed and 1,119 skipped. All 12 test/build tasks ran fresh with zero cached.
  • All 62 focused tests passed, including the original 24. Independent review accepted the finite lifecycle contract and callback-reuse controls.
  • Eighteen source/runtime comparisons preserve the reported safe pattern before and after promise resolution. Leaking variants retain warnings. Historical old-PR detector comparisons use current-main helper dependencies.
  • Separate fresh build, typecheck, lint, format check, schema-v3 JSON smoke, and published-package dependency checks passed.
  • All 7,493 source entries, staged index/tree, eight workspace links, and 169 built outputs stayed stable through normal signed commit hooks.
  • The staged self-scan returned exit 1 for five js-set-map-lookups warnings. All five messages and source contexts match main. The normal hooks completed successfully with their existing nonblocking Doctor step. Score fetches were blocked, so no score or target-rule CLI coverage is claimed.
  • All seven platforms passed naturally on attempt 1 in CI run 41546019087. Modern Linux/macOS passed 34,984 tests with 1,119 skipped. Node 20 passed 34,981 with 1,122 skipped. Windows passed 34,917 with 1,186 skipped. Each main test invocation ran five builds and seven test tasks with zero cached.
  • All nine observed workflows passed. Hosted lint reported 807 warnings and zero errors. Typecheck ran ten tasks with zero cached.
  • The Doctor workflow succeeded with its scan skipped and no score. This provides no executed diagnostic coverage.

Limits

Iterator/helper initiation, complex producer flow, and aliases of declared API receivers remain outside this proof. The existing forEach missed-warning case remains recorded. External implementations must honor their declared API contracts.

Opt-in fuzz, local RDE, and corpus parity were not run. Package manifests remain 0.9.22.

… nullable cleanup (#1957)

- Extend hasGuardedDeferredCleanup to handle subscriptions in addition to timers
- Support subscriptions created in promise callbacks and cleaned up conditionally
- Recognize LogicalExpression guards (e.g. subscription && subscription.remove())
- Add comprehensive tests for async subscription cleanup patterns
- Add regression test in fuzz corpus

Closes #1957

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

pkg-pr-new Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 41da305

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Interactive terminal E2E

Terminal Control verified the built CLI at 41da305 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

@aidenybai
aidenybai marked this pull request as ready for review October 21, 2026 11:21
@aidenybai aidenybai changed the title fix: effect-needs-cleanup false positive for async subscriptions with nullable cleanup (#1957) FPFNFIXAPEX fix guarded async subscription cleanup Nov 2, 2026

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.

effect-needs-cleanup: false positive when the subscription is created in an async callback and removed in cleanup (0.9.17)

3 participants