From 01bf42897a8e8f94e47b5df7d44ae406855e92f7 Mon Sep 17 00:00:00 2001 From: bohdansolovie <153934212+bohdansolovie@users.noreply.github.com> Date: Sun, 5 Jul 2026 17:26:56 +0200 Subject: [PATCH 1/2] feat(enrichment): add floating-promise local analyzer Fixes #2023 Co-authored-by: Cursor --- .env.example | 8 +- apps/gittensory-ui/src/lib/rees-analyzers.ts | 23 +++ review-enrichment/analyzer-metadata.json | 27 ++++ .../src/analyzers/floating-promise.ts | 133 ++++++++++++++++++ review-enrichment/src/analyzers/registry.ts | 30 ++++ review-enrichment/src/render.ts | 1 + review-enrichment/src/types.ts | 9 ++ .../test/analyzer-registry.test.ts | 1 + .../test/floating-promise.test.ts | 83 +++++++++++ src/review/enrichment-analyzer-names.ts | 1 + 10 files changed, 312 insertions(+), 4 deletions(-) create mode 100644 review-enrichment/src/analyzers/floating-promise.ts create mode 100644 review-enrichment/test/floating-promise.test.ts diff --git a/.env.example b/.env.example index dc561a2300..8d35aba374 100644 --- a/.env.example +++ b/.env.example @@ -68,25 +68,25 @@ GITTENSORY_REVIEW_ENRICHMENT=false # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber -# conflictMarker,debugLeftover,sizeSmell,commitLint +# conflictMarker,debugLeftover,sizeSmell,floatingPromise,commitLint # # Profile defaults: # fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild # testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker -# debugLeftover,sizeSmell +# debugLeftover,sizeSmell,floatingPromise # balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript # heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight # typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication # churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch # commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology -# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,commitLint +# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,commitLint # deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber -# conflictMarker,debugLeftover,sizeSmell,commitLint +# conflictMarker,debugLeftover,sizeSmell,floatingPromise,commitLint # END GENERATED REES ANALYZERS # Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep diff --git a/apps/gittensory-ui/src/lib/rees-analyzers.ts b/apps/gittensory-ui/src/lib/rees-analyzers.ts index 730e7c655f..c81f2f350d 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -981,6 +981,29 @@ export const REES_ANALYZERS = [ "File length is estimated from hunk headers (the visible patch), not a full checkout. Function detection is structural (`function` / arrow-with-brace) and counts added body lines until brace balance returns to zero.", }, }, + { + name: "floatingPromise", + title: "Floating promises", + category: "quality", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: { + maxFindings: 25, + maxLineChars: 2000, + maxCallChars: 40, + }, + docs: { + summary: + "Flags newly-added promise-shaped calls whose returned promise is neither awaited, returned, voided, nor same-line .then/.catch-chained.", + looksAt: "Added lines in changed non-test TS/JS source files.", + reports: "File, line, and a truncated callee name (fetch, Promise.*, or *Async suffix).", + network: "Pure local analyzer. No external network call.", + notes: + "Precision-first: bare expression statements only — assignments and non-promise callees are skipped. Structural heuristic, not a type checker.", + }, + }, { name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index 27e07e1ed7..76f7797b96 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -1107,6 +1107,33 @@ "notes": "File length is estimated from hunk headers (the visible patch), not a full checkout. Function detection is structural (`function` / arrow-with-brace) and counts added body lines until brace balance returns to zero." } }, + { + "name": "floatingPromise", + "title": "Floating promises", + "category": "quality", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": { + "maxFindings": 25, + "maxLineChars": 2000, + "maxCallChars": 40 + }, + "docs": { + "summary": "Flags newly-added promise-shaped calls whose returned promise is neither awaited, returned, voided, nor same-line .then/.catch-chained.", + "looksAt": "Added lines in changed non-test TS/JS source files.", + "reports": "File, line, and a truncated callee name (fetch, Promise.*, or *Async suffix).", + "network": "Pure local analyzer. No external network call.", + "notes": "Precision-first: bare expression statements only — assignments and non-promise callees are skipped. Structural heuristic, not a type checker." + } + }, { "name": "commitLint", "title": "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/floating-promise.ts b/review-enrichment/src/analyzers/floating-promise.ts new file mode 100644 index 0000000000..92844a3d7c --- /dev/null +++ b/review-enrichment/src/analyzers/floating-promise.ts @@ -0,0 +1,133 @@ +// Floating-promise analyzer (#2023). Flags newly-added async-shaped calls whose returned promise is neither +// awaited, returned, voided, nor .catch()/.then()-chained on the same statement — a common silent-failure bug. +// Precision-first structural heuristic over added TS/JS lines only: promise-shaped callees (`fetch`, `Promise.*`, +// or an `*Async` suffix) on bare expression statements. Pure compute, no network. +import type { EnrichRequest, FloatingPromiseFinding } from "../types.js"; +import { codeOnly } from "./secret-log.js"; +import { isTestPath } from "./test-ratio.js"; + +const MAX_FINDINGS = 25; +const MAX_LINE_CHARS = 2000; +const MAX_CALL_CHARS = 40; + +const JS_TS_PATH_RE = /\.(?:tsx?|jsx?|mts|cts|cjs|mjs)$/i; + +const HANDLED_PREFIX = + /^\s*(?:await\b|return\b|void\b|throw\b|if\b|for\b|while\b|switch\b|case\b|else\b|try\b|catch\b|finally\b|import\b|export\b|const\b|let\b|var\b|type\b|interface\b|class\b|function\b|async\s+function\b)/; + +const PROMISE_CHAIN_RE = /\.(?:then|catch|finally)\s*\(/; + +function isJsTsPath(path: string): boolean { + return JS_TS_PATH_RE.test(path) && !isTestPath(path); +} + +function isCommentLine(line: string): boolean { + const trimmed = line.trimStart(); + return /^(?:\/\/|\/\*|\*)/.test(trimmed); +} + +function truncateCall(call: string): string { + if (call.length <= MAX_CALL_CHARS) return call; + return `${call.slice(0, MAX_CALL_CHARS - 3)}...`; +} + +function isPromiseShapedCallee(callee: string): boolean { + if (callee === "fetch" || callee.endsWith(".fetch")) return true; + if (callee === "Promise" || /^Promise\.(?:all(?:Settled)?|race|any|resolve|reject)$/.test(callee)) { + return true; + } + const last = callee.split(".").pop() ?? callee; + return /Async$/.test(last); +} + +function extractLeadingCallCallee(line: string): string | null { + const code = codeOnly(line).trim(); + const semiIdx = code.indexOf(";"); + if (semiIdx >= 0 && semiIdx < code.length - 1) { + const after = code.slice(semiIdx + 1).trim(); + if (after.length > 0) return null; + } + + const newPromise = /^new\s+Promise\s*\(/.exec(code); + if (newPromise) return "Promise"; + + const match = /^((?:[a-zA-Z_$][\w$]*(?:\.[a-zA-Z_$][\w$]*)*))\s*\(/.exec(code); + return match?.[1] ?? null; +} + +/** Classify one added line for a floating promise call, or null. Pure. */ +export function detectFloatingPromise(line: string): string | null { + if (isCommentLine(line) || HANDLED_PREFIX.test(line) || PROMISE_CHAIN_RE.test(line)) { + return null; + } + + const code = codeOnly(line).replace(/=>/g, " "); + if (/(?!])=(?!=)/.test(code)) return null; + + const callee = extractLeadingCallCallee(line); + if (!callee || !isPromiseShapedCallee(callee)) return null; + + return truncateCall(callee); +} + +type ScanLimits = { + maxFindings?: number; + signal?: AbortSignal; +}; + +/** Scan one file patch's added lines for floating promises, line-cited via hunk headers. Pure. */ +export function scanPatchForFloatingPromise( + path: string, + patch: string, + limits: ScanLimits = {}, +): FloatingPromiseFinding[] { + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0 || !isJsTsPath(path)) return []; + const findings: FloatingPromiseFinding[] = []; + let newLine = 0; + let inHunk = false; + for (const line of patch.split("\n")) { + if (limits.signal?.aborted) throw new Error("analyzer_aborted"); + const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (hunk) { + newLine = Number(hunk[1]); + inHunk = true; + continue; + } + if (!inHunk) continue; + if (line.startsWith("+")) { + const body = line.slice(1); + if (body.length <= MAX_LINE_CHARS) { + const call = detectFloatingPromise(body); + if (call) { + findings.push({ file: path, line: newLine, call }); + if (findings.length >= maxFindings) return findings; + } + } + newLine++; + } else if (!line.startsWith("-") && !line.startsWith("\\")) { + newLine++; + } + } + return findings; +} + +/** Analyzer entrypoint: scan every changed TS/JS file's added lines for floating promises. */ +export async function scanFloatingPromise( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: FloatingPromiseFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForFloatingPromise(file.path, file.patch, { + maxFindings: MAX_FINDINGS - findings.length, + signal, + })) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index 914e52f4b6..4cac2b05e3 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -32,6 +32,7 @@ import { scanLooseRanges } from "./loose-range.js"; import { scanMagicNumbers } from "./magic-number.js"; import { scanConflictMarkers } from "./conflict-marker.js"; import { scanDebugLeftover } from "./debug-leftover.js"; +import { scanFloatingPromise } from "./floating-promise.js"; import { scanSizeSmell } from "./size-smell.js"; import { scanCommitLint } from "./commit-lint.js"; import { scanTerminology } from "./terminology.js"; @@ -1046,6 +1047,35 @@ export const ANALYZER_DESCRIPTORS = [ }, run: (req, { signal }) => scanSizeSmell(req, signal), }), + descriptor({ + name: "floatingPromise", + title: "Floating promises", + category: "quality", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxLineChars: 2000, maxCallChars: 40 }, + docs: { + summary: + "Flags newly-added promise-shaped calls whose returned promise is neither awaited, returned, voided, nor same-line .then/.catch-chained.", + looksAt: "Added lines in changed non-test TS/JS source files.", + reports: "File, line, and a truncated callee name (fetch, Promise.*, or *Async suffix).", + network: "Pure local analyzer. No external network call.", + notes: + "Precision-first: bare expression statements only — assignments and non-promise callees are skipped. Structural heuristic, not a type checker.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Floating promises (async call not awaited/returned/chained)"]; + for (const item of findings) { + lines.push( + `- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.call)}`, + ); + } + return lines; + }, + run: (req, { signal }) => scanFloatingPromise(req, signal), + }), descriptor({ name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 08486ae5e5..4185cc7c17 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -483,6 +483,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker)); lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover)); lines.push(...renderDescriptorSection("sizeSmell", findings.sizeSmell)); + lines.push(...renderDescriptorSection("floatingPromise", findings.floatingPromise)); lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl)); lines.push(...renderDescriptorSection("commitLint", findings.commitLint)); diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 6a7af486f2..de18137576 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -494,6 +494,14 @@ export interface SizeSmellFinding { name?: string; } +/** A promise-shaped call added without await/return/void or a same-line .then/.catch chain (#2023, part of #1499). + * Reports location and a truncated callee name — never full expressions. */ +export interface FloatingPromiseFinding { + file: string; + line: number; + call: string; +} + /** An absolute HTTP(S) URL or raw IP:port endpoint hardcoded in non-test, non-config source (#2027, part of #1499). * Reports location, kind, and a redacted/truncated host — never full paths or query strings. */ export interface HardcodedUrlFinding { @@ -551,6 +559,7 @@ export interface BriefFindings { conflictMarker?: ConflictMarkerFinding[]; debugLeftover?: DebugLeftoverFinding[]; sizeSmell?: SizeSmellFinding[]; + floatingPromise?: FloatingPromiseFinding[]; hardcodedUrl?: HardcodedUrlFinding[]; commitLint?: CommitLintFinding[]; } diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 3e4d06849f..64a000c8ff 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -49,6 +49,7 @@ const EXPECTED_ANALYZERS = [ "conflictMarker", "debugLeftover", "sizeSmell", + "floatingPromise", "commitLint", ]; diff --git a/review-enrichment/test/floating-promise.test.ts b/review-enrichment/test/floating-promise.test.ts new file mode 100644 index 0000000000..eea59af369 --- /dev/null +++ b/review-enrichment/test/floating-promise.test.ts @@ -0,0 +1,83 @@ +// Units for the floating-promise analyzer (#2023). Own file so concurrent analyzer PRs don't collide. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + detectFloatingPromise, + scanFloatingPromise, + scanPatchForFloatingPromise, +} from "../dist/analyzers/floating-promise.js"; +import { renderBrief } from "../dist/render.js"; + +const patchOf = (lines: string[]) => + `@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`; + +test("detectFloatingPromise: flags bare promise-shaped calls", () => { + assert.equal(detectFloatingPromise("loadUserAsync();"), "loadUserAsync"); + assert.equal(detectFloatingPromise(" service.saveAsync(payload);"), "service.saveAsync"); + assert.equal(detectFloatingPromise("fetch('/api/users');"), "fetch"); + assert.equal(detectFloatingPromise("Promise.all(items.map(runAsync));"), "Promise.all"); + assert.equal(detectFloatingPromise("new Promise((resolve) => resolve(1));"), "Promise"); +}); + +test("detectFloatingPromise: does not flag awaited, returned, voided, or chained calls", () => { + assert.equal(detectFloatingPromise("await loadUserAsync();"), null); + assert.equal(detectFloatingPromise("return await loadUserAsync();"), null); + assert.equal(detectFloatingPromise("return loadUserAsync();"), null); + assert.equal(detectFloatingPromise("void fetch('/health');"), null); + assert.equal(detectFloatingPromise("fetch('/x').catch(() => {});"), null); + assert.equal(detectFloatingPromise("fetch('/x').then(handleOk);"), null); +}); + +test("detectFloatingPromise: skips assignments, non-promise calls, and comments", () => { + assert.equal(detectFloatingPromise("const user = loadUserAsync();"), null); + assert.equal(detectFloatingPromise("console.log('hi');"), null); + assert.equal(detectFloatingPromise("saveUser(user);"), null); + assert.equal(detectFloatingPromise("// await loadUserAsync();"), null); +}); + +test("scanPatchForFloatingPromise: flags added lines with correct locations", () => { + const findings = scanPatchForFloatingPromise( + "src/worker.ts", + patchOf([ + "export function run() {", + " syncSetup();", + " flushQueueAsync();", + "}", + ]), + ); + assert.deepEqual(findings, [{ file: "src/worker.ts", line: 3, call: "flushQueueAsync" }]); +}); + +test("scanPatchForFloatingPromise: skips test files and non-JS/TS paths", () => { + assert.deepEqual( + scanPatchForFloatingPromise("src/worker.test.ts", patchOf(["loadUserAsync();"])), + [], + ); + assert.deepEqual( + scanPatchForFloatingPromise("lib/worker.py", patchOf(["load_user_async()"])), + [], + ); +}); + +test("scanPatchForFloatingPromise: respects the findings cap", () => { + const lines = Array.from({ length: 30 }, (_, i) => `task${i}Async();`); + assert.equal(scanPatchForFloatingPromise("src/a.ts", patchOf(lines), { maxFindings: 3 }).length, 3); +}); + +test("scanFloatingPromise: aggregates across files and renders in the brief", async () => { + const findings = await scanFloatingPromise({ + files: [ + { path: "src/a.ts", patch: patchOf(["fetch('/api');"]) }, + { path: "src/b.ts", patch: patchOf(["syncJobAsync();"]) }, + ], + }); + assert.deepEqual(findings, [ + { file: "src/a.ts", line: 1, call: "fetch" }, + { file: "src/b.ts", line: 1, call: "syncJobAsync" }, + ]); + + const { promptSection } = renderBrief({ floatingPromise: findings }); + assert.match(promptSection, /Floating promises/); + assert.match(promptSection, /src\/a\.ts:1/); + assert.match(promptSection, /src\/b\.ts:1/); +}); diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index 00abaa4af1..1853764172 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -43,6 +43,7 @@ export const REES_ANALYZER_NAMES = [ "conflictMarker", "debugLeftover", "sizeSmell", + "floatingPromise", "commitLint", ] as const; From 74d81188aaac900a7114a5a5c7b590510940bcc8 Mon Sep 17 00:00:00 2001 From: bohdansolovie <153934212+bohdansolovie@users.noreply.github.com> Date: Sun, 5 Jul 2026 17:36:18 +0200 Subject: [PATCH 2/2] fix(enrichment): do not treat .finally() as handled promise chain `.finally()` does not consume rejection; only .then/.catch skip the finding. Co-authored-by: Cursor --- review-enrichment/src/analyzers/floating-promise.ts | 2 +- review-enrichment/test/floating-promise.test.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/review-enrichment/src/analyzers/floating-promise.ts b/review-enrichment/src/analyzers/floating-promise.ts index 92844a3d7c..b26000e93d 100644 --- a/review-enrichment/src/analyzers/floating-promise.ts +++ b/review-enrichment/src/analyzers/floating-promise.ts @@ -15,7 +15,7 @@ const JS_TS_PATH_RE = /\.(?:tsx?|jsx?|mts|cts|cjs|mjs)$/i; const HANDLED_PREFIX = /^\s*(?:await\b|return\b|void\b|throw\b|if\b|for\b|while\b|switch\b|case\b|else\b|try\b|catch\b|finally\b|import\b|export\b|const\b|let\b|var\b|type\b|interface\b|class\b|function\b|async\s+function\b)/; -const PROMISE_CHAIN_RE = /\.(?:then|catch|finally)\s*\(/; +const PROMISE_CHAIN_RE = /\.(?:then|catch)\s*\(/; function isJsTsPath(path: string): boolean { return JS_TS_PATH_RE.test(path) && !isTestPath(path); diff --git a/review-enrichment/test/floating-promise.test.ts b/review-enrichment/test/floating-promise.test.ts index eea59af369..d5e5ae5290 100644 --- a/review-enrichment/test/floating-promise.test.ts +++ b/review-enrichment/test/floating-promise.test.ts @@ -26,6 +26,7 @@ test("detectFloatingPromise: does not flag awaited, returned, voided, or chained assert.equal(detectFloatingPromise("void fetch('/health');"), null); assert.equal(detectFloatingPromise("fetch('/x').catch(() => {});"), null); assert.equal(detectFloatingPromise("fetch('/x').then(handleOk);"), null); + assert.equal(detectFloatingPromise("fetch('/x').finally(cleanup);"), "fetch"); }); test("detectFloatingPromise: skips assignments, non-promise calls, and comments", () => {