diff --git a/scripts/check-no-conflict-markers.d.mts b/scripts/check-no-conflict-markers.d.mts index ad1ceb752ca..984440cd201 100644 --- a/scripts/check-no-conflict-markers.d.mts +++ b/scripts/check-no-conflict-markers.d.mts @@ -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; -import fs from "node:fs"; diff --git a/scripts/check-no-conflict-markers.mjs b/scripts/check-no-conflict-markers.mjs index 807cc6cd6eb..2c9bf136293 100644 --- a/scripts/check-no-conflict-markers.mjs +++ b/scripts/check-no-conflict-markers.mjs @@ -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; } diff --git a/test/scripts/check-no-conflict-markers.test.ts b/test/scripts/check-no-conflict-markers.test.ts index dcd8b6ff234..f950a9a5a53 100644 --- a/test/scripts/check-no-conflict-markers.test.ts +++ b/test/scripts/check-no-conflict-markers.test.ts @@ -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`); + }); });