Skip to content

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

Closed
bohdansolovie wants to merge 2 commits into
JSONbored:mainfrom
bohdansolovie:feat/enrichment-floating-promise-2023-v2
Closed

feat(enrichment): floating-promise / unhandled-rejection analyzer#3524
bohdansolovie wants to merge 2 commits into
JSONbored:mainfrom
bohdansolovie:feat/enrichment-floating-promise-2023-v2

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
  • .finally() alone is not treated as handled (it does not consume rejection); covered by an explicit unit test
  • 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, .finally() regression, 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

bohdansolovie and others added 2 commits July 5, 2026 17:36
Fixes JSONbored#2023

Co-authored-by: Cursor <cursoragent@cursor.com>
`.finally()` does not consume rejection; only .then/.catch skip the finding.

Co-authored-by: Cursor <cursoragent@cursor.com>
@bohdansolovie
bohdansolovie requested a review from JSONbored as a code owner July 5, 2026 15:36
@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:39:57 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:58 checks `PROMISE_CHAIN_RE` against the raw `line`, so `fetch("/.catch(")
  • ` is treated as handled even though the production path has a bare floating `fetch`
  • run the chain check on `codeOnly(line)` instead, e.g. `const code = codeOnly(line).replace(/=>/g, " ")
  • if (isCommentLine(line) || HANDLED_PREFIX.test(line) || PROMISE_CHAIN_RE.test(code)) return null
  • ` and add that regression test. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This PR adds a local floating-promise analyzer, registers it through the REES registry/rendering/metadata paths, and covers the main happy paths with focused unit tests. The wiring is coherent and the issue link is present, but the analyzer checks the raw line for `.then`/`.catch` before stripping string literals, so a literal argument can hide a real floating promise despite the PR explicitly claiming string literals are stripped before matching.

Blockers

  • review-enrichment/src/analyzers/floating-promise.ts:58 checks `PROMISE_CHAIN_RE` against the raw `line`, so `fetch("/.catch(");` is treated as handled even though the production path has a bare floating `fetch`; run the chain check on `codeOnly(line)` instead, e.g. `const code = codeOnly(line).replace(/=>/g, " "); if (isCommentLine(line) || HANDLED_PREFIX.test(line) || PROMISE_CHAIN_RE.test(code)) return null;` and add that regression test.
Nits — 6 non-blocking
  • nit: review-enrichment/src/analyzers/floating-promise.ts:37 treats bare `Promise.resolve()` / `Promise.reject()` as actionable, which may create low-value findings for intentionally ignored settled promises; either narrow this to combinators/new Promise/fetch/*Async or document why these are worth reporting.
  • nit: review-enrichment/src/analyzers/floating-promise.ts:53 only rejects lines with a semicolon plus trailing text, so `fetch('/x') .catch(...)` on added adjacent lines will be reported even though it is handled; that may be acceptable for a line-only heuristic, but the docs should call out the same-line limitation more explicitly.
  • nit: review-enrichment/test/floating-promise.test.ts should include a string-literal regression for `.catch(`/`.then(` so the “string literals stripped before matching” contract stays covered.
  • In review-enrichment/src/analyzers/floating-promise.ts, compute the stripped `code` once at the top of `detectFloatingPromise` and use it for the handled-prefix, chain, assignment, and callee extraction checks so the analyzer has one parsing view.
  • In review-enrichment/test/floating-promise.test.ts, add `assert.equal(detectFloatingPromise("fetch('/.catch(');"), "fetch")` or an equivalent case before this ships.
  • 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:58 checks `PROMISE_CHAIN_RE` against the raw `line`, so `fetch("/.catch(");` is treated as handled even though the production path has a bare floating `fetch`; run the chain check on `codeOnly(line)` instead, e.g. `const code = codeOnly(line).replace(/=>/g, " "); if (isCommentLine(line) || HANDLED_PREFIX.test(line) || PROMISE_CHAIN_RE.test(code)) return null;` and add that regression test.
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: 318 registered-repo PR(s), 186 merged, 9 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bohdansolovie; Gittensor profile; 318 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: 318 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:58 checks `PROMISE_CHAIN_RE` against the raw `line`, so `fetch("/.catch(");` is treated as handled even though the production path has a bare floating `fetch`; run the chain check on `codeOnly(line)` instead, e.g. `const code = codeOnly(line).replace(/=>/g, " "); if (isCommentLine(line) || HANDLED_PREFIX.test(line) || PROMISE_CHAIN_RE.test(code)) return null;` and add that regression test.). 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 (fc021a5) to head (74d8118).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3524   +/-   ##
=======================================
  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