Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
27 changes: 27 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
133 changes: 133 additions & 0 deletions review-enrichment/src/analyzers/floating-promise.ts
Original file line number Diff line number Diff line change
@@ -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<FloatingPromiseFinding[]> {
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;
}
30 changes: 30 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
9 changes: 9 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -551,6 +559,7 @@ export interface BriefFindings {
conflictMarker?: ConflictMarkerFinding[];
debugLeftover?: DebugLeftoverFinding[];
sizeSmell?: SizeSmellFinding[];
floatingPromise?: FloatingPromiseFinding[];
hardcodedUrl?: HardcodedUrlFinding[];
commitLint?: CommitLintFinding[];
}
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/test/analyzer-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const EXPECTED_ANALYZERS = [
"conflictMarker",
"debugLeftover",
"sizeSmell",
"floatingPromise",
"commitLint",
];

Expand Down
83 changes: 83 additions & 0 deletions review-enrichment/test/floating-promise.test.ts
Original file line number Diff line number Diff line change
@@ -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/);
});
Loading
Loading