mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 02:06:43 +00:00
fix(scripts): bound conflict-marker scan reads to prevent OOM on large files (#104420)
* fix(scripts): bound conflict-marker scan reads to prevent OOM on large files * test(scripts): add regression test for bounded conflict-marker scan on large files * fix(scripts): update conflict-marker declaration for bounded scan params * fix(scripts): bounded exact scan for conflict markers * fix(scripts): use git grep line-number output for conflict-marker scan * style(scripts): rename shadowed variable in conflict-marker parser * ci(scripts): retry build artifacts after plugin-sdk export check flake * fix(scripts): report tracked conflict-marker paths relative to cwd * test(scripts): type execFileSync error by stderr shape * fix(scripts): parse NUL-delimited path before record split in conflict-marker scan * refactor(scripts): remove obsolete conflict scan helper * fix(scripts): disable color in conflict marker scan Co-authored-by: 陈宪彪0668000387 <chen.xianbiao@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: Peter Steinberger <peter@steipete.me>
This commit is contained in:
co-authored by
Peter Steinberger
Peter Steinberger
parent
b95035491c
commit
efa7e1a85a
@@ -2,30 +2,30 @@
|
||||
/**
|
||||
* Returns one-based line numbers containing merge conflict markers.
|
||||
*/
|
||||
export function findConflictMarkerLines(content: unknown): unknown[];
|
||||
export function findConflictMarkerLines(content: string): number[];
|
||||
/**
|
||||
* Lists tracked files in the repository.
|
||||
*/
|
||||
export function listTrackedFiles(cwd?: string): string[];
|
||||
/**
|
||||
* Scans files for merge conflict markers, skipping binary content.
|
||||
* Scans a list of files for merge conflict markers, skipping binary content.
|
||||
* Intended for direct/small-scale use; the tracked-files path uses git grep
|
||||
* directly to avoid reading large files into memory.
|
||||
*/
|
||||
export function findConflictMarkersInFiles(
|
||||
filePaths: unknown,
|
||||
readFile?: typeof fs.readFileSync,
|
||||
filePaths: string[],
|
||||
readFile?: (path: string) => string | Buffer,
|
||||
): {
|
||||
filePath: unknown;
|
||||
lines: unknown[];
|
||||
filePath: string;
|
||||
lines: number[];
|
||||
}[];
|
||||
/**
|
||||
* Finds merge conflict markers in tracked repository files.
|
||||
* Uses git grep to find exact conflict-marker matches in tracked files.
|
||||
*/
|
||||
export function findConflictMarkersInTrackedFiles(cwd?: string): {
|
||||
filePath: unknown;
|
||||
lines: unknown[];
|
||||
export function findConflictMarkersInTrackedFiles(
|
||||
cwd?: string,
|
||||
run?: typeof import("node:child_process").spawnSync,
|
||||
): {
|
||||
filePath: string;
|
||||
lines: number[];
|
||||
}[];
|
||||
/**
|
||||
* Runs the merge conflict marker check.
|
||||
*/
|
||||
export function main(): Promise<void>;
|
||||
import fs from "node:fs";
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
#!/usr/bin/env node
|
||||
|
||||
// Rejects unresolved merge conflict markers in tracked files.
|
||||
import { execFileSync, spawnSync } from "node:child_process";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
@@ -32,21 +32,9 @@ export function findConflictMarkerLines(content) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Lists tracked files in the repository.
|
||||
*/
|
||||
export function listTrackedFiles(cwd = process.cwd()) {
|
||||
const output = execFileSync("git", ["ls-files", "-z"], {
|
||||
cwd,
|
||||
encoding: "utf8",
|
||||
});
|
||||
return output
|
||||
.split("\0")
|
||||
.filter(Boolean)
|
||||
.map((relativePath) => path.join(cwd, relativePath));
|
||||
}
|
||||
|
||||
/**
|
||||
* Scans files for merge conflict markers, skipping binary content.
|
||||
* Scans a list of files for merge conflict markers, skipping binary content.
|
||||
* This is kept for direct, small-scale use (tests); the tracked-files path
|
||||
* uses git grep directly to avoid reading large files into memory.
|
||||
*/
|
||||
export function findConflictMarkersInFiles(filePaths, readFile = fs.readFileSync) {
|
||||
const violations = [];
|
||||
@@ -75,12 +63,66 @@ export function findConflictMarkersInFiles(filePaths, readFile = fs.readFileSync
|
||||
}
|
||||
|
||||
/**
|
||||
* Uses git grep to list tracked files that may contain conflict markers.
|
||||
* Parses output from `git grep -n -z -o -I -E` into violation records.
|
||||
* The record format is:
|
||||
* path\0line-number\0match-text\n
|
||||
* The path must be read from its NUL delimiter before any newline-based
|
||||
* record splitting: -z reports paths verbatim, and git allows newlines in
|
||||
* tracked filenames, so splitting the output on newlines first would cut a
|
||||
* newline-containing path across two records and silently mangle it.
|
||||
* Keeping parsing to exact grep match records avoids reading candidate files
|
||||
* whole and keeps memory bounded regardless of file size.
|
||||
*/
|
||||
function listTrackedFilesWithConflictMarkerCandidates(cwd = process.cwd(), run = spawnSync) {
|
||||
function parseGitGrepConflictMarkerOutput(stdout) {
|
||||
const byPath = new Map();
|
||||
const output = stdout.toString("utf8");
|
||||
let cursor = 0;
|
||||
|
||||
while (cursor < output.length) {
|
||||
const pathEnd = output.indexOf("\0", cursor);
|
||||
if (pathEnd === -1) {
|
||||
break;
|
||||
}
|
||||
const lineNumberEnd = output.indexOf("\0", pathEnd + 1);
|
||||
if (lineNumberEnd === -1) {
|
||||
break;
|
||||
}
|
||||
// With -o the match text is a single-line match, so the record ends at
|
||||
// the next newline after the line-number field.
|
||||
const matchEnd = output.indexOf("\n", lineNumberEnd + 1);
|
||||
const relativePath = output.slice(cursor, pathEnd);
|
||||
const lineNumber = Number(output.slice(pathEnd + 1, lineNumberEnd));
|
||||
cursor = matchEnd === -1 ? output.length : matchEnd + 1;
|
||||
if (!relativePath || !Number.isFinite(lineNumber) || lineNumber <= 0) {
|
||||
continue;
|
||||
}
|
||||
const existing = byPath.get(relativePath);
|
||||
if (existing) {
|
||||
existing.push(lineNumber);
|
||||
} else {
|
||||
byPath.set(relativePath, [lineNumber]);
|
||||
}
|
||||
}
|
||||
|
||||
const violations = [];
|
||||
for (const [relativePath, lineNumbers] of byPath) {
|
||||
lineNumbers.sort((a, b) => a - b);
|
||||
violations.push({
|
||||
filePath: relativePath,
|
||||
lines: lineNumbers,
|
||||
});
|
||||
}
|
||||
violations.sort((a, b) => a.filePath.localeCompare(b.filePath));
|
||||
return violations;
|
||||
}
|
||||
|
||||
/**
|
||||
* Uses git grep to find exact conflict-marker matches in tracked files.
|
||||
*/
|
||||
export function findConflictMarkersInTrackedFiles(cwd = process.cwd(), run = spawnSync) {
|
||||
const result = run(
|
||||
"git",
|
||||
["grep", "-l", "-z", "-I", "-E", CONFLICT_MARKER_GREP_PATTERN, "--", "."],
|
||||
["grep", "--no-color", "-n", "-z", "-o", "-I", "-E", CONFLICT_MARKER_GREP_PATTERN, "--", "."],
|
||||
{
|
||||
cwd,
|
||||
encoding: "buffer",
|
||||
@@ -93,18 +135,7 @@ function listTrackedFilesWithConflictMarkerCandidates(cwd = process.cwd(), run =
|
||||
const stderr = result.stderr?.toString("utf8").trim();
|
||||
throw new Error(stderr || `git grep failed with status ${result.status ?? "unknown"}`);
|
||||
}
|
||||
return result.stdout
|
||||
.toString("utf8")
|
||||
.split("\0")
|
||||
.filter(Boolean)
|
||||
.map((relativePath) => path.join(cwd, relativePath));
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds merge conflict markers in tracked repository files.
|
||||
*/
|
||||
export function findConflictMarkersInTrackedFiles(cwd = process.cwd()) {
|
||||
return findConflictMarkersInFiles(listTrackedFilesWithConflictMarkerCandidates(cwd));
|
||||
return parseGitGrepConflictMarkerOutput(result.stdout);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -119,8 +150,8 @@ export async function main() {
|
||||
|
||||
console.error("Found unresolved merge conflict markers:");
|
||||
for (const violation of violations) {
|
||||
const relativePath = path.relative(cwd, violation.filePath) || violation.filePath;
|
||||
console.error(`- ${relativePath}:${violation.lines.join(",")}`);
|
||||
// findConflictMarkersInTrackedFiles already returns paths relative to cwd.
|
||||
console.error(`- ${violation.filePath}:${violation.lines.join(",")}`);
|
||||
}
|
||||
process.exitCode = 1;
|
||||
}
|
||||
|
||||
@@ -7,7 +7,6 @@ import {
|
||||
findConflictMarkerLines,
|
||||
findConflictMarkersInFiles,
|
||||
findConflictMarkersInTrackedFiles,
|
||||
listTrackedFiles,
|
||||
} from "../../scripts/check-no-conflict-markers.mjs";
|
||||
import { createScriptTestHarness } from "./test-helpers.js";
|
||||
|
||||
@@ -63,16 +62,17 @@ describe("check-no-conflict-markers", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("finds conflict markers in tracked script files", () => {
|
||||
it("finds conflict markers in tracked files using git grep", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
|
||||
const scriptFile = path.join(rootDir, "scripts", "bundled-plugin-metadata-runtime.mjs");
|
||||
fs.mkdirSync(path.dirname(scriptFile), { recursive: true });
|
||||
const scriptFile = "scripts/bundled-plugin-metadata-runtime.mjs";
|
||||
const scriptPath = path.join(rootDir, scriptFile);
|
||||
fs.mkdirSync(path.dirname(scriptPath), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
scriptFile,
|
||||
scriptPath,
|
||||
[
|
||||
"<<<<<<< HEAD",
|
||||
'const left = "left";',
|
||||
@@ -81,14 +81,7 @@ describe("check-no-conflict-markers", () => {
|
||||
">>>>>>> branch",
|
||||
].join("\n"),
|
||||
);
|
||||
git(rootDir, "add", "scripts/bundled-plugin-metadata-runtime.mjs");
|
||||
|
||||
expect(findConflictMarkersInFiles(listTrackedFiles(rootDir))).toEqual([
|
||||
{
|
||||
filePath: scriptFile,
|
||||
lines: [1, 3, 5],
|
||||
},
|
||||
]);
|
||||
git(rootDir, "add", scriptFile);
|
||||
|
||||
const violations = findConflictMarkersInTrackedFiles(rootDir);
|
||||
|
||||
@@ -99,4 +92,178 @@ describe("check-no-conflict-markers", () => {
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("disables configured git grep colors before parsing records", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
git(rootDir, "config", "color.grep", "always");
|
||||
git(rootDir, "config", "color.grep.lineNumber", "red");
|
||||
|
||||
const conflictFile = "src/conflict.ts";
|
||||
const conflictPath = path.join(rootDir, conflictFile);
|
||||
fs.mkdirSync(path.dirname(conflictPath), { recursive: true });
|
||||
fs.writeFileSync(conflictPath, "<<<<<<< HEAD\nleft\n=======\nright\n>>>>>>> branch\n");
|
||||
git(rootDir, "add", conflictFile);
|
||||
|
||||
expect(findConflictMarkersInTrackedFiles(rootDir)).toEqual([
|
||||
{
|
||||
filePath: conflictFile,
|
||||
lines: [1, 3, 5],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("returns no violations when tracked files have no conflict markers", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
|
||||
const cleanFile = "src/clean.ts";
|
||||
const cleanPath = path.join(rootDir, cleanFile);
|
||||
fs.mkdirSync(path.dirname(cleanPath), { recursive: true });
|
||||
fs.writeFileSync(cleanPath, "const x = 1;\n");
|
||||
git(rootDir, "add", cleanFile);
|
||||
|
||||
expect(findConflictMarkersInTrackedFiles(rootDir)).toEqual([]);
|
||||
});
|
||||
|
||||
it("skips binary tracked files via git grep binary exclusion", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
|
||||
const binaryFile = "assets/image.png";
|
||||
const binaryPath = path.join(rootDir, binaryFile);
|
||||
fs.mkdirSync(path.dirname(binaryPath), { recursive: true });
|
||||
// Marker-like bytes inside a binary file should not be reported because
|
||||
// git grep -I skips binary files.
|
||||
fs.writeFileSync(
|
||||
binaryPath,
|
||||
Buffer.from([0x3c, 0x3c, 0x3c, 0x3c, 0x3c, 0x3c, 0x3c, 0x20, 0x00]),
|
||||
);
|
||||
git(rootDir, "add", binaryFile);
|
||||
|
||||
expect(findConflictMarkersInTrackedFiles(rootDir)).toEqual([]);
|
||||
});
|
||||
|
||||
it("handles tracked files with spaces and unusual characters in paths", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
|
||||
const weirdFile = "docs/weird name (v2).md";
|
||||
const weirdPath = path.join(rootDir, weirdFile);
|
||||
fs.mkdirSync(path.dirname(weirdPath), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
weirdPath,
|
||||
"before\n<<<<<<< HEAD\nleft\n=======\nright\n>>>>>>> branch\nafter\n",
|
||||
);
|
||||
git(rootDir, "add", weirdFile);
|
||||
|
||||
const violations = findConflictMarkersInTrackedFiles(rootDir);
|
||||
|
||||
expect(violations).toEqual([
|
||||
{
|
||||
filePath: weirdFile,
|
||||
lines: [2, 4, 6],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("reports tracked filenames containing newlines without mangling the path", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
|
||||
// git allows newlines in tracked filenames and git grep -z prints the
|
||||
// path verbatim; the parser must read the NUL-delimited path before any
|
||||
// newline-based record splitting or the path is silently truncated.
|
||||
const newlineFile = "docs/weird\nname.md";
|
||||
const newlinePath = path.join(rootDir, newlineFile);
|
||||
fs.mkdirSync(path.dirname(newlinePath), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
newlinePath,
|
||||
"before\n<<<<<<< HEAD\nleft\n=======\nright\n>>>>>>> branch\nafter\n",
|
||||
);
|
||||
git(rootDir, "add", newlineFile);
|
||||
|
||||
const violations = findConflictMarkersInTrackedFiles(rootDir);
|
||||
|
||||
expect(violations).toEqual([
|
||||
{
|
||||
filePath: newlineFile,
|
||||
lines: [2, 4, 6],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("detects markers in a file larger than the previous scan byte limit without reading it whole", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
|
||||
const largeFile = "generated/large.txt";
|
||||
const largePath = path.join(rootDir, largeFile);
|
||||
fs.mkdirSync(path.dirname(largePath), { recursive: true });
|
||||
// 10 MiB of filler with a marker near the end; git grep reports the line
|
||||
// number without us buffering the entire file.
|
||||
const filler = ("a".repeat(10240) + "\n").repeat(1024);
|
||||
fs.writeFileSync(largePath, filler + "<<<<<<< HEAD\nleft\n=======\nright\n>>>>>>> branch\n");
|
||||
git(rootDir, "add", largeFile);
|
||||
|
||||
const violations = findConflictMarkersInTrackedFiles(rootDir);
|
||||
|
||||
expect(violations).toEqual([
|
||||
{
|
||||
filePath: largeFile,
|
||||
lines: [1025, 1027, 1029],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("main reports tracked violations with paths relative to cwd", () => {
|
||||
const rootDir = createTempDir("openclaw-conflict-markers-main-");
|
||||
git(rootDir, "init", "-q");
|
||||
git(rootDir, "config", "user.email", "test@example.com");
|
||||
git(rootDir, "config", "user.name", "Test User");
|
||||
|
||||
const conflictFile = "src/conflict.ts";
|
||||
const conflictPath = path.join(rootDir, conflictFile);
|
||||
fs.mkdirSync(path.dirname(conflictPath), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
conflictPath,
|
||||
[
|
||||
"<<<<<<< HEAD",
|
||||
'const value = "left";',
|
||||
"=======",
|
||||
'const value = "right";',
|
||||
">>>>>>> branch",
|
||||
].join("\n"),
|
||||
);
|
||||
git(rootDir, "add", conflictFile);
|
||||
|
||||
const scriptPath = path.resolve(__dirname, "../../scripts/check-no-conflict-markers.mjs");
|
||||
let error: Error | undefined;
|
||||
try {
|
||||
execFileSync(process.execPath, [scriptPath], {
|
||||
cwd: rootDir,
|
||||
encoding: "utf8",
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
} catch (caught) {
|
||||
error = caught as Error;
|
||||
}
|
||||
|
||||
expect(error).toBeDefined();
|
||||
const stderr = (error as { stderr?: string }).stderr ?? "";
|
||||
expect(stderr).toContain("Found unresolved merge conflict markers:");
|
||||
expect(stderr).toContain(`- ${conflictFile}:1,3,5`);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user