Skip to content

Commit a979552

Browse files
feat(enrichment): leftover console.log / debugger analyzer (#3472)
* feat(enrichment): add debug-leftover analyzer for console.log and debugger Fixes #2015 — flags plain debug leftovers in non-test source separately from secret-log. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(enrichment): restrict print() debug detection to Python paths Avoid false positives on method calls like document.print(). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(enrichment): require non-method print() calls in Python debug scan Use (?<![\w.])print so obj.print() and document.print() are not flagged. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 4fd8c9d commit a979552

9 files changed

Lines changed: 279 additions & 4 deletions

File tree

.env.example

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,23 +67,24 @@ GITTENSORY_REVIEW_ENRICHMENT=false
6767
# provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig,nativeBuild
6868
# history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity,ciCheckSignals
6969
# undocumentedExport,staleBranch,commitHygiene,pendingReviewRequests,testRatio,migrationSafety
70-
# looseRange,terminology,todoMarker,magicNumber,conflictMarker,commitLint
70+
# looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover,commitLint
7171
#
7272
# Profile defaults:
7373
# fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol
7474
# redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,testRatio,migrationSafety
75-
# looseRange,terminology,todoMarker,magicNumber,conflictMarker
75+
# looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover
7676
# balanced (default): dependency,lockfileDrift,secret,license,installScript,heavyDependency
7777
# actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature
7878
# iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink
7979
# approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
8080
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
81-
# conflictMarker,commitLint
81+
# conflictMarker,debugLeftover,commitLint
8282
# deep: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol
8383
# redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig
8484
# nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity
8585
# ciCheckSignals,undocumentedExport,staleBranch,commitHygiene,pendingReviewRequests,testRatio
86-
# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,commitLint
86+
# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover
87+
# commitLint
8788
# END GENERATED REES ANALYZERS
8889

8990
# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep

apps/gittensory-ui/src/lib/rees-analyzers.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,28 @@ export const REES_ANALYZERS = [
886886
"Structural: an exactly-seven-character marker run at column 0. The ambiguous `=======` separator is not flagged in Markdown/AsciiDoc files, where it is a legitimate section rule.",
887887
},
888888
},
889+
{
890+
name: "debugLeftover",
891+
title: "Debug leftovers",
892+
category: "quality",
893+
cost: "local",
894+
defaultEnabled: true,
895+
profiles: ["fast", "balanced", "deep"],
896+
requires: ["files"],
897+
limits: {
898+
maxFindings: 25,
899+
maxLineChars: 2000,
900+
},
901+
docs: {
902+
summary:
903+
"Flags debugging leftovers a PR adds in non-test source — `debugger;`, bare console sinks, or `print()` calls.",
904+
looksAt: "Added lines in changed non-test source files.",
905+
reports: "File, line, and kind: debugger, console, or print.",
906+
network: "Pure local analyzer. No external network call.",
907+
notes:
908+
"Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.",
909+
},
910+
},
889911
{
890912
name: "commitLint",
891913
title: "Conventional-commit subjects",

review-enrichment/analyzer-metadata.json

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,32 @@
999999
"notes": "Structural: an exactly-seven-character marker run at column 0. The ambiguous `=======` separator is not flagged in Markdown/AsciiDoc files, where it is a legitimate section rule."
10001000
}
10011001
},
1002+
{
1003+
"name": "debugLeftover",
1004+
"title": "Debug leftovers",
1005+
"category": "quality",
1006+
"cost": "local",
1007+
"defaultEnabled": true,
1008+
"profiles": [
1009+
"fast",
1010+
"balanced",
1011+
"deep"
1012+
],
1013+
"requires": [
1014+
"files"
1015+
],
1016+
"limits": {
1017+
"maxFindings": 25,
1018+
"maxLineChars": 2000
1019+
},
1020+
"docs": {
1021+
"summary": "Flags debugging leftovers a PR adds in non-test source — `debugger;`, bare console sinks, or `print()` calls.",
1022+
"looksAt": "Added lines in changed non-test source files.",
1023+
"reports": "File, line, and kind: debugger, console, or print.",
1024+
"network": "Pure local analyzer. No external network call.",
1025+
"notes": "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching."
1026+
}
1027+
},
10021028
{
10031029
"name": "commitLint",
10041030
"title": "Conventional-commit subjects",
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Debug-leftover analyzer (#2015). Flags debugging leftovers introduced in the diff — `debugger;` statements
2+
// and bare `console.*` / `print()` calls added to non-test source files. Distinct from the secret-log analyzer
3+
// (which only fires on sensitive-value sinks); this catches plain debug noise regardless of payload. Pure compute,
4+
// no network. String-literal content is stripped before matching so a `"console.log('hi')"` inside a string is
5+
// not flagged. Line-cited via hunk headers, mirroring the sibling local analyzers.
6+
import type { DebugLeftoverFinding, EnrichRequest } from "../types.js";
7+
import { codeOnly } from "./secret-log.js";
8+
import { isTestPath } from "./test-ratio.js";
9+
10+
const MAX_FINDINGS = 25;
11+
const MAX_LINE_CHARS = 2000;
12+
13+
const DEBUGGER_RE = /\bdebugger\s*;/;
14+
const CONSOLE_RE = /\bconsole\s*\.\s*(?:log|debug|info|warn|error|trace|dir|table)\s*\(/;
15+
const PRINT_RE = /(?<![\w.])print\s*\(/;
16+
17+
/** Classify one added line for a debug leftover, or null. Pure. */
18+
export function detectDebugLeftover(
19+
line: string,
20+
path?: string,
21+
): DebugLeftoverFinding["kind"] | null {
22+
const code = codeOnly(line);
23+
if (DEBUGGER_RE.test(code)) return "debugger";
24+
if (CONSOLE_RE.test(code)) return "console";
25+
// Python-only: `\bprint` after a dot would false-positive on `document.print()` / `obj.print()`.
26+
if (path && /\.pyi?$/i.test(path) && PRINT_RE.test(code)) return "print";
27+
return null;
28+
}
29+
30+
type ScanLimits = {
31+
maxFindings?: number;
32+
signal?: AbortSignal;
33+
};
34+
35+
/** Scan one file patch's added lines for debug leftovers, line-cited via hunk headers. Pure. */
36+
export function scanPatchForDebugLeftover(
37+
path: string,
38+
patch: string,
39+
limits: ScanLimits = {},
40+
): DebugLeftoverFinding[] {
41+
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
42+
if (maxFindings <= 0 || isTestPath(path)) return [];
43+
const findings: DebugLeftoverFinding[] = [];
44+
let newLine = 0;
45+
let inHunk = false;
46+
for (const line of patch.split("\n")) {
47+
if (limits.signal?.aborted) throw new Error("analyzer_aborted");
48+
const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
49+
if (hunk) {
50+
newLine = Number(hunk[1]);
51+
inHunk = true;
52+
continue;
53+
}
54+
if (!inHunk) continue;
55+
if (line.startsWith("+")) {
56+
const body = line.slice(1);
57+
if (body.length <= MAX_LINE_CHARS) {
58+
const kind = detectDebugLeftover(body, path);
59+
if (kind) {
60+
findings.push({ file: path, line: newLine, kind });
61+
if (findings.length >= maxFindings) return findings;
62+
}
63+
}
64+
newLine++;
65+
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
66+
newLine++;
67+
}
68+
}
69+
return findings;
70+
}
71+
72+
/** Analyzer entrypoint: scan every changed non-test file's added lines for debug leftovers. */
73+
export async function scanDebugLeftover(
74+
req: EnrichRequest,
75+
signal?: AbortSignal,
76+
): Promise<DebugLeftoverFinding[]> {
77+
const findings: DebugLeftoverFinding[] = [];
78+
for (const file of req.files ?? []) {
79+
if (signal?.aborted) throw new Error("analyzer_aborted");
80+
if (!file.patch) continue;
81+
for (const finding of scanPatchForDebugLeftover(file.path, file.patch, {
82+
maxFindings: MAX_FINDINGS - findings.length,
83+
signal,
84+
})) {
85+
findings.push(finding);
86+
if (findings.length >= MAX_FINDINGS) return findings;
87+
}
88+
}
89+
return findings;
90+
}

review-enrichment/src/analyzers/registry.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { scanMigrationSafety } from "./migration-safety.js";
2929
import { scanLooseRanges } from "./loose-range.js";
3030
import { scanMagicNumbers } from "./magic-number.js";
3131
import { scanConflictMarkers } from "./conflict-marker.js";
32+
import { scanDebugLeftover } from "./debug-leftover.js";
3233
import { scanCommitLint } from "./commit-lint.js";
3334
import { scanTerminology } from "./terminology.js";
3435
import { scanTodoMarker } from "./todo-marker.js";
@@ -917,6 +918,35 @@ export const ANALYZER_DESCRIPTORS = [
917918
},
918919
run: (req) => scanConflictMarkers(req),
919920
}),
921+
descriptor({
922+
name: "debugLeftover",
923+
title: "Debug leftovers",
924+
category: "quality",
925+
cost: "local",
926+
defaultEnabled: true,
927+
requires: ["files"],
928+
limits: { maxFindings: 25, maxLineChars: 2000 },
929+
docs: {
930+
summary:
931+
"Flags debugging leftovers a PR adds in non-test source — `debugger;`, bare console sinks, or `print()` calls.",
932+
looksAt: "Added lines in changed non-test source files.",
933+
reports: "File, line, and kind: debugger, console, or print.",
934+
network: "Pure local analyzer. No external network call.",
935+
notes:
936+
"Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.",
937+
},
938+
render: (findings, helpers) => {
939+
if (!findings.length) return [];
940+
const lines = ["### Debug leftovers (debugger / console / print added by this PR)"];
941+
for (const item of findings) {
942+
lines.push(
943+
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)}${helpers.safeCodeSpan(item.kind)}`,
944+
);
945+
}
946+
return lines;
947+
},
948+
run: (req, { signal }) => scanDebugLeftover(req, signal),
949+
}),
920950
descriptor({
921951
name: "commitLint",
922952
title: "Conventional-commit subjects",

review-enrichment/src/render.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ export function renderBrief(
480480
lines.push(...renderDescriptorSection("todoMarker", findings.todoMarker));
481481
lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber));
482482
lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker));
483+
lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover));
483484
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));
484485

485486
if (!lines.length) return { promptSection: "", systemSuffix: "" };

review-enrichment/src/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,14 @@ export interface ConflictMarkerFinding {
465465
marker: "<<<<<<<" | "|||||||" | "=======" | ">>>>>>>";
466466
}
467467

468+
/** A debugging leftover a PR added in non-test source — `debugger;`, a bare console sink, or a `print()` call
469+
* (#2015, part of #1499). Distinct from secret-log (sensitive payloads); reports location + kind only. */
470+
export interface DebugLeftoverFinding {
471+
file: string;
472+
line: number;
473+
kind: "debugger" | "console" | "print";
474+
}
475+
468476
/** A PR commit subject that does not conform to the Conventional Commits spec (#2021, part of #1499). Reports a
469477
* short SHA prefix, the subject, and the failing reason — never author/email. */
470478
export interface CommitLintFinding {
@@ -510,6 +518,7 @@ export interface BriefFindings {
510518
todoMarker?: TodoMarkerFinding[];
511519
magicNumber?: MagicNumberFinding[];
512520
conflictMarker?: ConflictMarkerFinding[];
521+
debugLeftover?: DebugLeftoverFinding[];
513522
commitLint?: CommitLintFinding[];
514523
}
515524

review-enrichment/test/analyzer-registry.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ const EXPECTED_ANALYZERS = [
4545
"todoMarker",
4646
"magicNumber",
4747
"conflictMarker",
48+
"debugLeftover",
4849
"commitLint",
4950
];
5051

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Units for the debug-leftover analyzer (#2015). Own file (not enrichment.test.ts) so concurrent analyzer PRs
2+
// don't collide. No network — pure, stateless per-line detection. Runs against the compiled dist/.
3+
import { test } from "node:test";
4+
import assert from "node:assert/strict";
5+
import {
6+
detectDebugLeftover,
7+
scanDebugLeftover,
8+
scanPatchForDebugLeftover,
9+
} from "../dist/analyzers/debug-leftover.js";
10+
import { renderBrief } from "../dist/render.js";
11+
12+
const patchOf = (lines: string[]) =>
13+
`@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`;
14+
15+
test("detectDebugLeftover: recognizes debugger, console sinks, and print()", () => {
16+
assert.equal(detectDebugLeftover(" debugger;"), "debugger");
17+
assert.equal(detectDebugLeftover("console.log('hi')"), "console");
18+
assert.equal(detectDebugLeftover(" console.debug(state)"), "console");
19+
assert.equal(detectDebugLeftover("print('debug')", "lib/b.py"), "print");
20+
});
21+
22+
test("detectDebugLeftover: print() is Python-only and does not match method calls like document.print()", () => {
23+
assert.equal(detectDebugLeftover("document.print()"), null);
24+
assert.equal(detectDebugLeftover("printer.print('x')"), null);
25+
assert.equal(detectDebugLeftover("print('debug')", "src/widget.ts"), null);
26+
assert.equal(detectDebugLeftover("obj.print('x')", "pkg/widget.py"), null);
27+
});
28+
29+
test("detectDebugLeftover: a console call inside a string literal is not flagged", () => {
30+
assert.equal(detectDebugLeftover('const s = "console.log(\\"nope\\")"'), null);
31+
assert.equal(detectDebugLeftover("log(`hint: console.log(here)`);"), null);
32+
});
33+
34+
test("detectDebugLeftover: debugger inside a string is not flagged", () => {
35+
assert.equal(detectDebugLeftover('const msg = "debugger;"'), null);
36+
});
37+
38+
test("scanPatchForDebugLeftover: flags added lines with correct locations", () => {
39+
const findings = scanPatchForDebugLeftover(
40+
"src/widget.ts",
41+
patchOf(["function f() {", " debugger;", " console.log('x');", " return g();", "}"]),
42+
);
43+
assert.deepEqual(findings, [
44+
{ file: "src/widget.ts", line: 2, kind: "debugger" },
45+
{ file: "src/widget.ts", line: 3, kind: "console" },
46+
]);
47+
});
48+
49+
test("scanPatchForDebugLeftover: only ADDED lines are scanned", () => {
50+
const patch = [
51+
"@@ -10,2 +10,2 @@",
52+
" function f() {",
53+
"- console.log('old');",
54+
"+ print('new')",
55+
].join("\n");
56+
assert.deepEqual(scanPatchForDebugLeftover("pkg/widget.py", patch), [
57+
{ file: "pkg/widget.py", line: 11, kind: "print" },
58+
]);
59+
});
60+
61+
test("scanPatchForDebugLeftover: skips test/spec files", () => {
62+
assert.deepEqual(
63+
scanPatchForDebugLeftover("src/widget.test.ts", patchOf(["console.log('in test')"])),
64+
[],
65+
);
66+
assert.deepEqual(
67+
scanPatchForDebugLeftover("tests/widget.spec.js", patchOf(["debugger;"])),
68+
[],
69+
);
70+
});
71+
72+
test("scanPatchForDebugLeftover: respects the findings cap", () => {
73+
const lines = Array.from({ length: 30 }, (_, i) => `console.log(${i});`);
74+
assert.equal(scanPatchForDebugLeftover("src/a.ts", patchOf(lines), { maxFindings: 3 }).length, 3);
75+
});
76+
77+
test("scanDebugLeftover: aggregates across files and renders in the brief", async () => {
78+
const findings = await scanDebugLeftover({
79+
files: [
80+
{ path: "src/a.ts", patch: patchOf(["debugger;"]) },
81+
{ path: "lib/b.py", patch: patchOf(["print('x')"]) },
82+
],
83+
});
84+
assert.deepEqual(findings, [
85+
{ file: "src/a.ts", line: 1, kind: "debugger" },
86+
{ file: "lib/b.py", line: 1, kind: "print" },
87+
]);
88+
89+
const { promptSection } = renderBrief({
90+
debugLeftover: findings,
91+
});
92+
assert.match(promptSection, /Debug leftovers/);
93+
assert.match(promptSection, /src\/a\.ts:1/);
94+
assert.match(promptSection, /lib\/b\.py:1/);
95+
});

0 commit comments

Comments
 (0)