Skip to content

feat(enrichment): floating-promise / unhandled-rejection analyzer#3521

Closed
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:feat/enrichment-floating-promise-2023
Closed

feat(enrichment): floating-promise / unhandled-rejection analyzer#3521
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:feat/enrichment-floating-promise-2023

Conversation

@bohdansolovie

Copy link
Copy Markdown
Contributor

Summary

Fixes #2023

  • Add FloatingPromiseFinding and a local floatingPromise analyzer that flags newly-added promise-shaped calls (fetch, Promise.*, new Promise, or *Async suffix) when the statement is neither awaited, returned, voided, nor same-line .then/.catch-chained
  • Precision-first: TS/JS non-test files only, bare expression statements (assignments skipped), string literals stripped before matching, capped at 25 findings
  • Register the analyzer in the REES registry with render/docs/metadata; sync src/review/enrichment-analyzer-names.ts and generated .env.example / UI metadata
  • Add review-enrichment/test/floating-promise.test.ts covering flagged calls, safe handled variants, non-JS skip, cap, and brief rendering

Test plan

  • cd review-enrichment && npm run build && npm run metadata && node --test test/floating-promise.test.ts test/analyzer-registry.test.ts
  • CI validate-code

Made with Cursor

Fixes JSONbored#2023

Co-authored-by: Cursor <cursoragent@cursor.com>
@bohdansolovie
bohdansolovie requested a review from JSONbored as a code owner July 5, 2026 15:27
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-05 15:28:12 UTC

10 files · 1 AI reviewer · 1 blocker · readiness 62/100 · CI pending · blocked

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/floating-promise.ts:17 treats `.finally()` as handled via `PROMISE_CHAIN_RE`, so `fetch('/x').finally(cleanup)
  • ` is skipped even though the returned promise can still reject unhandled
  • change it to `/\.(?:then|catch)\s*\(/` or tell me why `.finally()` alone is safe here. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This adds a local REES analyzer for newly-added promise-shaped expression statements and wires it through registry metadata, rendering, env docs, UI metadata, and targeted tests. The broad integration shape is correct and the issue link is present, but the detector currently treats `.finally()` as a handled promise chain even though `.finally()` does not consume rejection, so it misses a reachable unhandled-rejection case the analyzer is meant to catch.

Blockers

  • review-enrichment/src/analyzers/floating-promise.ts:17 treats `.finally()` as handled via `PROMISE_CHAIN_RE`, so `fetch('/x').finally(cleanup);` is skipped even though the returned promise can still reject unhandled; change it to `/\.(?:then|catch)\s*\(/` or tell me why `.finally()` alone is safe here.
Nits — 5 non-blocking
  • nit: review-enrichment/src/analyzers/floating-promise.ts:43 should have a unit test for `.finally()` specifically so this distinction does not regress.
  • nit: review-enrichment/src/analyzers/floating-promise.ts:54 uses a nested unbounded regex on added source text; the 2000-character cap limits exposure, but a small hand parser for dotted identifiers would be easier to reason about.
  • Add a test like `assert.equal(detectFloatingPromise("fetch('/x').finally(cleanup);"), "fetch");` in `review-enrichment/test/floating-promise.test.ts`.
  • Keep the docs/render wording aligned with the implementation after the `.finally()` change; the current summary already says `.then/.catch`, which is the right contract.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.

Why this is blocked

  • review-enrichment/src/analyzers/floating-promise.ts:17 treats `.finally()` as handled via `PROMISE_CHAIN_RE`, so `fetch('/x').finally(cleanup);` is skipped even though the returned promise can still reject unhandled; change it to `/\.(?:then|catch)\s*\(/` or tell me why `.finally()` alone is safe here.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #2023
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 312 registered-repo PR(s), 185 merged, 9 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bohdansolovie; Gittensor profile; 312 PR(s), 9 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: bohdansolovie
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 312 PR(s), 9 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (issue #2023, issue #2029)
  • Related work: Titles/paths share 7 meaningful terms. (issue #2023, issue #2033)
  • Related work: Titles/paths share 7 meaningful terms. (issue #2023, issue #2026)
Contributor next steps
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/floating-promise.ts:17 treats `.finally()` as handled via `PROMISE_CHAIN_RE`, so `fetch('/x').finally(cleanup);` is skipped even though the returned promise can still reject unhandled; change it to `/\.(?:then|catch)\s*\(/` or tell me why `.finally()` alone is safe here.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.09%. Comparing base (371a96c) to head (d84e067).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3521   +/-   ##
=======================================
  Coverage   93.09%   93.09%           
=======================================
  Files         301      301           
  Lines       31450    31450           
  Branches    11483    11483           
=======================================
  Hits        29279    29279           
  Misses       1517     1517           
  Partials      654      654           
Files with missing lines Coverage Δ
src/review/enrichment-analyzer-names.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(enrichment): floating-promise / unhandled-rejection analyzer

1 participant